Word Search solving algorithm The following is my wordsearch solving algorithm. I'd greatly appreciate tips on how to improve it (e.g. improve efficiency, better approach, etc.):

public boolean traverse(Grid grid) {
    for (int r = 0; r < grid.getRows(); r++) {
        for (int c = 0; c < grid.getCols(); c++) {
            for (Direction direction : Direction.values()) {
                currentPoint.y = r;
                currentPoint.x = c;
                this.direction = direction;

                if (find(grid, currentPoint, 0)) {
                    for (Point point : consideredPoints) {
                        grid.markLetterAt(point);
                    }

                   return true;
                }
            }
        }
    }

    return false;
}

Direction is an enum with all the cardinal directions (N, NE, S, NW, etc...). It has a method (updatePoint()) which changes a Point to the direction (e.g. NE does point.y -= 1, point.x += 1)

Here is find:

private boolean find(Grid grid, Point point, int count) {
    if (!grid.isValidLetter(point)) { // Checks if the point is valid (e.g not < 0)
        consideredPoints.clear();
        return false;
    }

    if (grid.getLetterAt(point).equals(word.substring(count, count + 1))) {
        consideredPoints.add(new Point(point));

        if (count == word.length() - 1) {
            return true;
        }

        direction.updatePoint(point);
        return find(grid, point, count + 1);
    }

    consideredPoints.clear();
    return false;
}

You are creating lots of Point() objects. For performance optimization in Java the very first thing is to avoid pressure on the garbage collector.
You are doing lots of repeated calculations just for traversing the grid (in several directions). Performance wise it would be much better to transform the whole grid in advance to simplify the traversal calculation. E.g. it is very simple to search only words in East direction in a grid. And searching words in West direction could then be done by mirroring the grid against a vertical axis and searching again in East direction only.
For just finding words, you can ignore the grid structure entirely. Take the complete first row as String, append a space, append the complete second row as String and so forth. You get one huge string representing the grid for searching in East direction. Your matching code is simply hugeString.contains(searchWord) (with a bit more complicated calculation of the original row and column information). You can do the same transformation from grid to a single String also for all other directions.


Prime factors kata solution in Clojure I'm new to Clojure but I already know about functions and lists, so I decided to implement Prime Factors Kata solution. For any natural number the function should return a list of all its prime factors, e.g. for 15 it should return (3 5).

It turned out that to implement a stack safe function I needed to use loop and recur, which I was not familiar with.

So, does my solution look like idiomatic Clojure? What could be improved? Thanks!

(defn primeFactorsOf
 "Return all prime factors of a number"
 [n]
 (loop [number n candidate 2 factors ()]
   (if (= number 1)
     (reverse factors)
     (if (= (mod number candidate) 0)
       (recur (/ number candidate) candidate (conj factors candidate))
       (recur number (inc candidate) factors)))))

There's a few notable things here:

You're prepending to a list, then reversing the result at the end. It would be better to just start with a vector ([]). conjoining to a vector appends the element to the end instead, so you don't need to reverse it after.

If you really want a list here because it's checking for type, then either ignore this suggestion, or put this code into a helper and do something like:

(def primeFactorsOf [n]
  (list* (helper n)))

I personally think a vector makes more sense here, but it may prove to not be worth it in this case.

(= (mod number candidate) 0)

can just be

(zero? (mod number candidate))

I wouldn't have all your loop accumulators on the same line like you have. It's much harder to read, since you have to manually group them in two's in your head to make sense of them. I'd split them up:

(loop [number n
       candidate 2
       factors []]
  . . .)

If you really want to have them on one line though, add some commas in there. Commas have, as far as I know, no purpose other to allow you to group expressions to help readability. I'd add them in every two here:

(loop [number n, candidate 2, factors []]
  . . .)

Nested if expressions are usually cleaner using a cond. There are a multitude of ways to format a cond block, but when you have long lines like you have here, I like to format it almost like you would an if statement in a imperative language (below).

All together, I end up with:

(defn primeFactorsOf
 "Return all prime factors of a number"
 [n]
 (loop [number n
        candidate 2
        factors []]
   (cond
     (= number 1)
     factors

     (zero? (mod number candidate))
     (recur (/ number candidate) candidate (conj factors candidate))

     :else
     (recur number (inc candidate) factors))))


MVC Controller in Java Swing Apps - Singleton or public static This article has left me a little confused.

In explaining a method on how to design MVC into Java Swing apps the author defines the controller class as Singleton and relays a series of calls to the Controller for each operation.

In a view object:

...
Controller.getInstance().setOwner(this);
Controller.getInstance().setNameTextField(this.nameTextField);
Controller.getInstance().setEmailTextField(this.emailTextField);
Controller.getInstance().processForm();
...

And inside the controller class:

public void processForm() {
    result1 = doSomethingWithName(this.nameTextField.getText());
    result2 = doSomethingWithEmail(this.emailTextField.getText());
    this.owner.setResult(result1, result2);
}

I don't understand why this has been made so complex! Wouldn't it be simpler, less error prone and more modular to simply:

Controller.processForm(this, this.nameTextField.getText(), this.emailTextField.getText());

and

public static void processForm(OwnerClass owner, String name, String email) {
    result1 = doSomethingWithName(name);
    result2 = doSomethingWithEmail(email);
    this.owner.setResult(result1, result2);
}

Is anyone familiar with this design pattern? Is there reason to this method?

It's been a long time ago since I last used an MVC approach in Swing, but I immediately notice some things I wouldn't do.

Don't use a singleton, singletons are evil!
Passing view components to the controller? When I do MVC, I tend to pass the controller to my view elements, preferably just an interface. The view can request actions on the controller. This allows for total decoupling of the view. So you can also switch to something else than Swing. This probably isn't the 'default' approach of MVC with Swing however.

So yeah, it seems I would agree with you. Perhaps try looking for other MVC examples with Swing. This article doesn't look particularly appealing IMHO.


Parallel and Sequential array looping with async/await Are these good implementations of parallel and sequential "extension" methods, and do they accomplish what I think they do?

My understanding is that the first one is parallel because the await applies to each invocation of the async lambda that is passed to map. So each invocation will block asynchronously and run in parallel.

In contrast, the await in the sequential function applies to the outer function, so each iteration of the for loop will block, and the next iteration will only occur once the previous one completes.

I based these implementations on the answer found at this SO question: https://stackoverflow.com/questions/37576685/using-async-await-with-a-foreach-loop

Array.prototype.forEachParallel = async function (
   func: (item: any) => Promise<void>
): Promise<void> {
   await Promise.all(this.map(async (item: any) => await func(item)));
};

Array.prototype.forEachSequential = async function (
   func: (item: any) => Promise<void>
): Promise<void> {
   for (let item of this)
      await func(item);
};

Example usage:

await someList.forEachParallel(item => this.SomeAsyncOperation(item));

A little additional code is:

// type declaration
interface Array<T> {
   ...
   forEachParallel(func: (item: T) => Promise<void>): Promise<void>;
   forEachSequential(func: (item: T) => Promise<void>): Promise<void>;
}

// make sure nothing is already defined on these members
[
   ...
   Array.prototype.forEachParallel,
   Array.prototype.forEachSequential
].forEach(maybeMember => { if (maybeMember) throw new Error("prototype extension collision"); });

JavaScript's execution model

  My understanding is that the first one is parallel because the await applies to each invocation of the async lambda that is passed to map.

This is not quite right since JavaScript (i.e. ECMAScript) is not multi-threaded.

Simply speaking, the VM executing the code has a queue from which it dequeues an action (e.g. "run this file") and executes it. This is done ad infinitum. It runs every dequeued element either to completion, to a yield (in generator functions), an await (in async functions) or an iterator.next() (on a generator function).
The only way your code can be interfered with (preempted) is using these statements. But even then no parallelism is created, the VM just switches the current queue item it is processing, e.g. to the caller of iterator.next() when the generator function just yielded a value.

I recommend you to read Concurrency model and Event Loop on MDN, which explains it in more detail and formally better than I can off the top of my head :)

The effects of the execution model

Endless loops cannot be avoided

The following code will print Processed 1 and then hang in the endless loop.

await [1, 2, 3].forEachParallel(async (element: number) => {
  if (element === 2) {
    while (true);
  }
  else {
    console.log('Processed ' + element);
  }
});

Your forEachParallel will call Array#map, which will iterate the array's elements one-by-one and apply (i.e. call, not await!) the passed function on it. It is now useful to mentally desugar your passed function from the aync/await syntactic sugar:

async (item: any) => await func(item)
// <=> (just return the Promise)
async (item: any) => func(item)
// <=> (an async function without awaits does not need to be async anymore)
(item: any) => func(item)

After desugaring, it is now apparent that this is the reason why Array#map will hang on the second iteration.

forEachParallel vs. forEachSequential

  In contrast, the await in the sequential function applies to the outer function, so each iteration of the for loop will block, and the next iteration will only occur once the previous one completes.

Exactly!

forEachSequential guarantees that func will be called and completed in-order on the array elements. In contrast, forEachParallel guarantees as well that func will be called in-order on the array elements, but not that they complete in-order, too!

await [1, 2, 3].forEachParallel(async (element: number) => {
  console.log('Processed ' + element);
});

The code above might print:

Processed 3
Processed 1
Processed 2

Whereas the following code will always print 1, 2, 3 in-order:

await [1, 2, 3].forEachSequential(async (element: number) => {
  console.log('Processed ' + element);
});

// This will print 1, 2, 3 as well, necessitating 1000ms for *each*
// element.
await [1, 2, 3].forEachSequential(async (element: number) => {
  return new Promise(resolve => {
    setTimeout(() => {
      console.log('Processed ' + element);
      resolve();
    }, 1000);
  });
});

Quiz: forEachParallel with timeouts

Will the following code hang? If yes, where does it happen (in map, in await Promise.all(), in neither)? What does it print?

await [1, 2, 3].forEachParallel(async (element: number) => {
  setTimeout(() => {
    if (element === 2) {
      while (true);
    }
    else {
      console.log('Processed ' + element);
    }
  }, 0);
});

   Processed 1 will be printed and then the program will hang.

 setTimeout(..., 0) immediately adds the function to the end of the VM's queue (see the MDN link above). Since Array#map works sequentially on all elements, the VM's queue will look like
 [current: forEachParallel] | setTimeout from first element | setTimeout from the second element | setTimeout from the third element

 Actually, I am not really sure where it will hang. I suppose it might happen in forEachParallel (because await Promise.all(...)) will pave the way for the execution of the setTimeout callbacks (and thereby escpecially for the second one) or it might happen after forEachParallel has entirely completed its execution. Note that the promise returned by the function passed to it are resolved directly at the end through the implicit return, not in the setTimeout callback!

Design Suggestions

I would opt to not use async for the function you pass to Array#map:

await Promise.all(this.map(async (item: any) => await func(item)));

The first time I read the code, I thought: "Is this really the Array#map function which is passed an async function? But how should that work? Array#map does not deal with async functions, or does it?"

Put another way, I usually expect functions getting async functions passed as arguments to be aware of them and deal with them in a special way. Here, you are just using the fact that Array#map accepts a general and generic (element: T) => U function, where you happen to instantiate U with Promise<void>.

You can also leave out the type annotation, this results in the following line:

await Promise.all(this.map(item => func(item));
You are correctly using the generic type inside the Array interface, that's good!
In the implementation, however, you are leaving them out and thus lose type safety in the implementation. The code still benefits from the interface type safety, though.
I am not entirely sure about the best practices on this, but simply annotating the implementation as well worked flawlessly for me:

Array.prototype.forEachParallel = async function<T>(this: Array<T>, func: (item: T) => Promise<void>): Promise<void> {
  // TypeScript now correctly infers the result from this.map
  await Promise.all(this.map(item => func(item)));
};

Array.prototype.forEachSequential = async function<T>(func: (item: T) => Promise<void>) {
  /* ... */
};


Data and vtables vs. objects and methods I present a bunch of grids of data to the user, and want to sum certain columns in some of the grids.

// for one such grid:

rows = [{id: 0, name: "Alice", age: "23"},
        {id: 1, name: "Bob", age: "25"} /* ... */]

sum_aggregator = {
    init: function() { return 0 },
    step: function(l, r) { return l + parse(r) },
    stop: function(x) { return x }
}

columns = [{field: 'age', aggregator: sum_aggregator, filter: /* ... */}, /* ... */]

function reduce() { /* pseudocode */
    for each column c: totals[c] = columns[c].aggregator.init()
    for each row r: for each column c:
        totals[c] = columns[c].aggregator.step(totals[c], get_field(r, c))
    for each column c: totals[c] = columns[c].aggregator.stop(totals[c])
    return totals[c]
}

Using vtables---objects containing only functions---such as sum_aggregator seems unidiomatic javascript to me.  I should note that I have several other vtable'y things dealing with the rows (SQL'ish WHERE clauses in filter, etc.), and I'm sorta' jerry-rigging a sum-of-vtables class-like thingy in the columns array.

Is this good design?  Are there benefits to factoring it otherwise?  Gluing the vtable together using smaller building blocks seems like it does something good for concern separation, but using objects with methods seems more javascripty to me.  What are some good ways of thinking in this design space?

This is the first time I have heard "vtabels", the more known term is decision table. Also, on the contrary, using decision tables is very idiomatic JavaScript and is used often, just rarely with lambdas and more often with named functions.

Also, your rows array (is this a global intentionally?) uses rows implying it's a row where every element is a single object.

That said, JavaScript has functional methods like Array.prototype.map, Array.prototype.filter and Array.prototype.reduce.

You can use those to map, filter and reduce things.

For example, your code (assuming it sums ages) could be written as:

var rows = [{id: 0, name: "Alice", age: "23"},
            {id: 1, name: "Bob", age: "25"} /* ... */];

var ageSum = rows.reduce(function(accum,next){return (+next.age)+accum},0);
ageSum; //48

Given the more functional style, your SQL WHERE becomes .filter

rows WHERE name = 'Bob'

Becomes:

rows.filter(function(name){
    return name==="Bob";
})

Similarly, aggregators become .reduce and mappings (and joins) become .map.


Handling large images efficiently (to avoid OutOfMemoryError) After lots of attempts thanks to all the Android enthusiasts, I've found a method to handle large images efficiently in Android Applications in order to get rid of OutOfMemoryError. But I really need to know is this the correct formal way of doing this? Do I need to add some more, or should any changes be made? Please comment.

Bitmap bitmap1;

iv1 = (ImageView) findViewById(R.id.iv1);
iv2 = (ImageView) findViewById(R.id.iv2);
iv3 = (ImageView) findViewById(R.id.iv3);

BitmapFactory.Options o = new BitmapFactory.Options();
o.inJustDecodeBounds = true;

BitmapFactory.decodeResource(getResources(), R.drawable.image1);
BitmapFactory.decodeResource(getResources(), R.drawable.image2);
BitmapFactory.decodeResource(getResources(), R.drawable.image3);

int srcWidth = o.outWidth;
int srcHeight = o.outHeight;
int dstWidth = 600;
 int dstHeight = 450;
int inSampleSize = 1;
if (srcHeight > dstHeight || srcWidth > dstWidth) {
            final int halfHeight = srcHeight / 2;
            final int halfWidth = srcWidth / 2;
            while ((halfHeight / inSampleSize) > dstWidth
                    && (halfWidth / inSampleSize) > dstWidth) {
                inSampleSize *= 2;
            }
        }
        o.inSampleSize = inSampleSize;
        o.inJustDecodeBounds = false;
        bitmap1 = BitmapFactory.decodeResource(getResources(), R.drawable.image1, o);
        iv1.setImageBitmap(bitmap1);

        bitmap1 = BitmapFactory.decodeResource(getResources(), R.drawable.image2, o);
        iv2.setImageBitmap(bitmap1);
        Toast.makeText(getApplicationContext(),
                "DEcoded", Toast.LENGTH_LONG).show();

        bitmap1 = BitmapFactory.decodeResource(getResources(), R.drawable.image3, o);
        iv3.setImageBitmap(bitmap1);

    @Override
    protected void onDestroy() {
        bitmap1.recycle();
        super.onDestroy();
        Toast.makeText(getApplicationContext(),
                "Bitmap Destroyed", Toast.LENGTH_LONG).show();
    }

This looks like a bug:

while ((halfHeight / inSampleSize) > dstWidth
        && (halfWidth / inSampleSize) > dstWidth) {

You probably meant this way:

while ((halfHeight / inSampleSize) > dstHeight
        && (halfWidth / inSampleSize) > dstWidth) {

In onDestroy you call bitmap1.recycle(). One problem with that is bitmap1 might be null. The bigger problem is that you try to decode 3 bitmaps earlier, and bitmap1 will point to the last one, so you will recycle only the last one, neglecting the first two.

I'm not sure this is necessary at all, and I cannot find the relevant documentation. If it's really necessary to manually recycle all Bitmaps like this, then you need to store the references of all the Bitmaps you decoded, so that you can recycle on in onDestroy.

This looks a bit like magic, though mostly not your fault:

BitmapFactory.Options o = new BitmapFactory.Options();
o.inJustDecodeBounds = true;

BitmapFactory.decodeResource(getResources(), R.drawable.image1);
BitmapFactory.decodeResource(getResources(), R.drawable.image2);
BitmapFactory.decodeResource(getResources(), R.drawable.image3);

int srcWidth = o.outWidth;
int srcHeight = o.outHeight;

If I understand the docs correctly, you set o.inJustDecodeBounds = true so that you can run BitmapFactory.decodeResource(...) without allocating memory for images, so that you can query o.outWidth for image dimensions. If indeed such magic is going on, some comments would be nice.

I suspect that if you call BitmapFactory.decodeResource(...) 3 times, o.outWidth will have the width of only the last image. So you might want to rework this, extracting the main processing to a private method, and running it for each image separately.

Also in the docs, if decoding fails, then o.outWidth will be set to -1, and you might want to add some checks to handle that case gracefully.

This pattern is repeated 3 times:

bitmap1 = BitmapFactory.decodeResource(getResources(), R.drawable.image1, o);
iv1.setImageBitmap(bitmap1);

It would be good to extract this to a private method:

private void setImageBitmap(int viewId, int imageId, BitmapFactory.Options options) {
    Bitmap bitmap = BitmapFactory.decodeResource(getResources(), imageId, options);
    ImageView view = (ImageView) findViewById(viewId);
    view.setImageBitmap(bitmap);
}

Which you can call with:

setImageBitmap(int R.id.iv1, int R.drawable.image1, o);
setImageBitmap(int R.id.iv2, int R.drawable.image2, o);
setImageBitmap(int R.id.iv3, int R.drawable.image3, o);

If indeed you need to recycle the Bitmaps in onDestroy, then the new setImageBitmap helper is a good place to add the decoded Bitmap to a collection.


Algorithm for describing chess positions I have an interesting problem with my chess application. The application reads PGN files, and at any time while viewing a game from the file, the function below, when called, iterates over the chessboard, gets the square names for each piece, and then constructs a string containing the description of which piece is where.

The function runs without errors, however it has a slight issue with respect to its performance: When the chess board is full, or close to full with 32 pieces, it runs pretty fast. Once the pieces go off the board and there remains a handful of pieces to describe, the function returns with a delay of around half a second. I have not tested the function with performance tools, but one can feel there is something strange going on: given the length of the function and its algorithm (please don't swear reading it), I can't understand why it will run faster when the board is full and slower when it's close to empty.

Can anyone spot what causes this issue? In addition, what can I try to optimize it?

def pos_text():
    global node
    #Dictionary of piece symbols to store their location on the board
    d = {"K": [], "Q": [], "R": [], "B": [], "N": [], "P": [], "k": [], "q": [], "r":   [], "b": [], "n": [], "p": []}
    piece_letters = "KkQqRrNnBbPp"
    #Iterates over the board and assigns square names to the individual piece symbols in d
    for x in range(64):
        sq = str(node.board().piece_at(x))
        if sq in piece_letters:
            d[sq].append(chess.SQUARE_NAMES[x])
    # now makes a string containing the description of pieces:
    pos = "White pieces:\nKing: %s\n" % d["K"][0]
    if d["Q"]:
        pos += "Queens: %s\n" % ", ".join(d["Q"]) if len(d["Q"]) >= 2 else "Queen: %s\n" % d["Q"][0]
    if d["R"]:
        pos += "Rooks: %s\n" % ", ".join(d["R"]) if len(d["R"]) >= 2 else "Rook: %s\n" % d["R"][0]
    if d["B"]:
        pos += "Bishops: %s\n" % ", ".join(d["B"]) if len(d["B"]) >= 2 else "Bishop: %s\n" % d["B"][0]
    if d["N"]:
        pos += "Knights: %s\n" % ", ".join(d["N"]) if len(d["N"]) >= 2 else "Knight: %s\n" % d["N"][0]
    if d["P"]:
        pos += "Pawns: %s\n" % ", ".join(d["P"]) if len(d["P"]) >= 2 else "Pawn: %s\n" % d["P"][0]
    pos += "Black pieces:\nKing: %s\n" % d["k"][0]
    if d["q"]:
        pos += "Queens: %s\n" % ", ".join(d["q"]) if len(d["q"]) >= 2 else "Queen: %s\n" % d["q"][0]
    if d["r"]:
        pos += "Rooks: %s\n" % ", ".join(d["r"]) if len(d["r"]) >= 2 else "Rook: %s\n" % d["r"][0]
    if d["b"]:
        pos += "Bishops: %s\n" % ", ".join(d["b"]) if len(d["b"]) >= 2 else "Bishop: %s\n" % d["b"][0]
    if d["n"]:
        pos += "Knights: %s\n" % ", ".join(d["n"]) if len(d["n"]) >= 2 else "Knight: %s\n" % d["n"][0]
    if d["p"]:
        pos += "Pawns: %s\n" % ", ".join(d["p"]) if len(d["p"]) >= 2 else "Pawn: %s\n" % d["p"][0]
    return pos

The slowness when having fewer pieces

Having fewer pieces on the board will make this slower:

piece_letters = "KkQqRrNnBbPp"
for x in range(64):
    sq = str(node.board().piece_at(x))
    if sq in piece_letters:
        d[sq].append(chess.SQUARE_NAMES[x])

Consider these two (invalid) extremes:

When the board is full of K, the evaluation of sq in piece_letters will terminate with True after checking the first letter
When the board is empty, the evaluation of sq in piece_letters will have to check each letter in piece_letters until it can terminate with False

In the rest of the code,
there are more evaluations performed in the full case compared to the empty case,
but not enough to outweigh the effects of the sq in piece_letters check,
due to the length of piece_letters.

Changing piece_letters to this would help:

piece_letters = frozenset("KkQqRrNnBbPp")

This way the evaluation of sq in piece_letters won't have to step through all letters.

But actually, since you already have a dictionary with the possible pieces in d,
you don't need piece_letters at all,
you can use simply this:

for x in range(64):
    sq = str(node.board().piece_at(x))
    if sq in d:
        d[sq].append(chess.SQUARE_NAMES[x])

This is also better for another reason:
if d doesn't have a predefined [] value for the sq key,
then the original code wouldn't work anyway.
When you write it this way,
it's clear that d[sq] exists, and will not raise a KeyError,
so it's more coherent and readable.

Actually the piece_letters string can be useful to initialize d itself:

piece_letters = "KkQqRrNnBbPp"
d = dict((c, []) for c in piece_letters)

And d is a very poor name. pieces would be better.

Other optimizations

Avoid using global variables if possible.
It would be better to pass node.board() to the method if possible.
Actually it seems the method should belong to the board itself,
and I would rename it to print_board.

The printing of the pieces is very repetitive.
It would be better to extract their logic to a helper function, for example:

def format_piece(marker, single_name, plural_name):
    num = len(pieces[marker])
    if num:
        label = plural_name if num >= 2 else single_name
        return "%s: %s\n" % (label, ", ".join(pieces[marker]))
    return ''

pos = "White pieces:\nKing: %s\n" % d["K"][0]
pos += format_piece('Q', 'Queen', 'Queens')
pos += format_piece('R', 'Rook', 'Rooks')
# ...

This achieves 2 additional optimizations:

Count the length of each pieces[marker] only once
Avoid duplication in the formatting, by setting label


Erlang treasure hunt I have the following task:

  Create a simple treasure hunt game.

  Create a two-dimensional array of integers 10 by 10.  In a random
  position in the array store the number 1.  repeat  Get the user to
  enter coordinates where they think the treasure is.  If there is a 1
  at this position display ‘success’.  Until they find the treasure

Instead of using arrays I used lists:

-module(hunt).
-export([start/0]).
start() ->
    X = [[12,1,14,15,16,22,11,11,2,5],[12,13,14,15,16,22,11,11,2,5],[12,13,14,15,16,22,11,11,2,5],[12,13,14,15,16,22,11,11,2,5],[12,13,14,15,16,22,11,11,2,5],[12,13,14,15,16,22,11,11,2,5],[12,13,14,15,16,22,11,11,2,5],[12,13,14,15,16,22,11,11,2,5],[12,13,14,15,16,22,11,11,2,5],[12,13,14,15,16,22,11,11,2,5]],
    loop(X).
loop(X) ->
    Inputs = io:get_line("Enter the coordinates"),
    {A,_} = string:to_integer(string:substr(Inputs,1,2)),
    {B,_} = string:to_integer(string:substr(Inputs,3,2)),
    erlang:display(A),
    erlang:display(B),
    case get(X,A,B) of
        1 -> io:format("You did it!");
        _ -> io:format("Try again"), loop(X)
    end.
get(List,A,B) ->
    ghelper(List,A,B).
ghelper(List,A,B) -> ghelper(List,A,B,0,0).
ghelper([H|Tail],A,B,N,M) ->
    case N == A of
        true -> ghelper(second,H,B,M);
        false -> ghelper(Tail,A,B,N+1,M)
    end.
ghelper(second,[H|Tail],B,M) ->
    case M == B of
        true -> H;
        false -> ghelper(second,Tail,B,M+1)
    end.

Is there a way to improve this further? I know I could probably make a function to randomly generate a 2D array (represented by lists here), but apart from that, is there anything I could improve?

Even though your code works I have some comments regarding Erlang style and naming conventions:

Spaces after commas and empty lines between function definitions make the code a lot more readable.
Helper functions that do the actual work are usually named with the do_ prefix. Instead of ghelper I would have named that do_get.
Using lists:nth could greatly simplify your code. The standard library includes a lot of useful functions, I try to always check if there is an existing function for what I want to do and only if there's none then I implement it.


Project Euler problem 37: truncatable primes I am working on Project Euler problem 37 and I have the following program:

#!/usr/bin/ruby -w

require 'prime'
h=11
y=0
x=0
while x < 11
    if h.prime? == true
        boo = true
        f=0
        n=h
        c=0
            while n > 0
                f += (n%10) * 10**(10-c)
                n/=10
                boo = false if n.prime? == false && n != 0
                c+=1
            end
            n=h
            c=10
            while c > 0
            n = n % (10**c)
            boo = false if n.prime? == false && n != 0
            c-=1
            end
        f /= 10 while f % 10 == 0
        if boo == true
            y+=h
            x+=1
            p h
        end
    end
h+=2
end
p y

It's designed to catch all of the eleven truncatable primes and does a good job of it, it catches the first ten is under one second, but it takes forty-nine seconds to find the last one, which happens to be, admittedly, a much larger number than the rest, is there any way to speed this up so that the eleventh one does not take so much time?

Some small notes on style:

if h.prime? == true should be if h.prime?; never use method? == true or method? == false, use if and unless.
I would replace your if h.prime? with next unless h.prime? and unindent the entire inner block; I hate having the entire body of your loop wrapped in a single if statement.
You should really, really choose better variable names. Single character names are a terrible practice. This is borderline obfuscated, who wants to figure out what a blob of uncommented difficult to read code does, in order to help you improve its performance?

Here's a simple refactor which is (on my system) 4x faster than your code, mostly because I'm aborting the outer loop early if the first inner while fails:

#!/usr/bin/ruby -w

require 'prime'

h, y, x = 9, 0, 0

while x < 11
  h += 2
  next unless h.prime?

  boo, f, n, c = true, 0, h, 0

  while n > 0
    f += (n % 10) * 10 ** (10 - c)
    n /= 10
    boo = false if !n.prime? && n != 0
    c += 1
  end

  # Don't keep checking if we already know this number fails
  next unless boo

  n = h
  c = 10

  while c > 0
    n %= (10 ** c)
    boo = false if !n.prime? && n != 0
    c-=1
  end

  f /= 10 while f % 10 == 0

  if boo
    y += h
    x += 1
    p h
  end
end

p y


AngularJS to-do list controller I am new to Angular. Am I following best practices here? I understand controllers should not do too much. My controller seems to have a lot of logic here. Can I do anything to improve it? It is just a basic ToDo App.

var app = angular.module('app', []);

app.controller('FirstCtrl', function($scope, todos){
    $scope.todos = todos;

    $scope.todos.submit = function(){
        if ($scope.input == "" || $scope.input == undefined){
            return false;
        }
        else{
            $scope.todos.list.push({
                task:$scope.input,
                completed: false
            });
            $scope.input = "";
        }
    }

    $scope.clear = function(){
         $scope.todos.list = $scope.todos.list.filter(function(todo){
            return !todo.completed;
        });
    }

});

app.factory('todos', function(){
    var todos = {};

    todos.list = [];

    return todos;
})

app.directive('todo', function(){
    return {
        restrict: "E",
        templateUrl:'templates/todos.html',
        controller:'FirstCtrl as First'
    }
})

app.controller('FirstCtrl', function($scope, todos){

You're using the implicit dependency injection method which uses the argument name as the dependency name. If you will be minifying the code (which mangles the name, rendering this approach useless), you either have to use explicit dependency (ie. .controller(name, [dep1, dep2, ... fn]) or use ng-annotate.

Additionally, you should name your controller properly. Normally, what we do is append "Controller", "Service" etc. to the name. In this case, it should be like "ToDoController". Also prevents ambiguities in dependency injection.

if ($scope.input == "" || $scope.input == undefined){
  return false;
}

Angular has built-in validation. Usually, I call submit passing in form's valid property and the data. If the valid property of the form is true, then it is safe to assume that the data that came with it has passed Angular's validation.

$scope.todos.list.push({
  task:$scope.input,
  completed: false
});

This operation must be delegated to the factory, not in the controller. The issue is when you start to share data across different controllers. If you happen to have another section of the UI able to add new todos to the main list, then you'd be duplicating your push logic in another controller.

Delegating this to the factory will need you to expose a function that the controller can call, pass in the data, and the factory update the list accordingly. Since the controller has reference to the same list, changes will appear magically.

restrict: "E",

If you're targetting IE8, then you should be aware that "E" isn't an option. You'd have to use "A" and write attribute directives instead of element name directives.

controller:'FirstCtrl as First'

I would recommend that a directive have its own scope ("isolate scope" as they call it), and that the binding to the controller be explicit. My concept of "directives" are like "custom elements" - portable, self-contained code. One can easily write the following:

<to-do>
  <to-do-list list="todos"></to-do-list>
  <to-do-form onSubmit="addTodo"></to-do-form>
</to-do>

Where:

<to-do> is a wrapper, a directive with a transclude (allows inner html).
<to-do-list> is a directive that renders the to-do list, given an array to its list property.
<to-do-form> is a directive that renders an input, which calls a function assigned to onSubmit.

The controller will then look like this:

app.controller('TodoController', function($scope, ToDoFactory){

  // Get a reference to the ToDos and expose it to the template
  $scope.todos = ToDoFactory.getToDos();

  // Expose a way to call a function that adds todos
  $scope.addTodo = function(toDo, isValid){
    if(!isValid) return;
    ToDoFactory.add(toDo);
  }
});

The factory:

app.factory('ToDoFactory', function(){
  var todos = [];

  return {
    add: function(todo){
      todos.push(todo);
    },
    getToDos: function(){
      return todos;
    }
  }
});

The list be like:

app.directive('toDoList', function(){
  return {
    restrict: 'E',
    template: '

{{ todo }}

',
    scope: { list: '=' }
  }
});

The input be like:

app.directive('toDoForm', function(){
  return {
    restrict: 'E',
    template: '<form name="toDoForm"><input type="text" ng-model="todo" ...validations...><button type="submit" ng-click="onSubmit(todo, toDoForm.$valid)">Add</button></form>',
    scope: { onSubmit: '=' }
  }
});

Also, directives have a link function which can act as your setup area, much like a controller's function. This is helpful especially when setting up watches, data manipulations etc.


Generically Cache Database Lookups By Id For a project I am working on I had to query a database for a number of "child objects". Now in the mapping POJO for said child objects a reference to the parent is kept. That reference is available in the database as a foreign key constraint.

But if I retrieve a collection of all child elements they all map to vastly less (on the order of 1.5k:1) parent objects.
To prevent accessing the database for each and every parent object lookup I decided to cache the results of parent-object queries for the duration of child-object retrieval.

To accomplish this in a typesafe, but generic way I've written the following class:

public class LookupCache<T,P,R> {
    private final T delegate;

    private final Map<BiFunction<T, P, R>, Map<P, R>> cache = new HashMap<>();

    public static <T,P,R> LookupCache<T,P,R> forObject(T object) {
        return new LookupCache<>(object);
    }

    private LookupCache(T object) {
        this.delegate = object;
    }

    public R lookupOrCompute(BiFunction<T,P,R> compute, P param) {
        Map<P,R> cachedResults = cache.computeIfAbsent(compute, a -> new HashMap<>());
        return cachedResults.computeIfAbsent(param, p -> compute.apply(delegate, p));
    }
}

The usage is pretty simple:

try (ResultSet dbResult = connection.createStatement().executeQuery("SELECT id, parent_id, property FROM child;")) {
    final BiFunction<ParentDAO, Long, Parent> parentLookup = ParentDAO::getById;
    final LookupCache<ParentDAO, Long, Parent> lookupCache = LookupCache.forObject(new ParentDAOImpl());
    List<Child> result = new ArrayList<>();
    while (dbResult.next()) {
        Parent p = lookupCache.lookupOrCompute(parentLookup, dbResult.getLong("parent_id"));
        result.add(new Child(dbResult.getLong("id")
            , Property.fromDbState(result.getLong("property"))
            , p));
    }
    return result;
} // catch clause and default return for error

Note that usage is not up for review here, what I'm significantly more interested in is:

Is this Cache designed in a way that's intuitive to use?
Am I possibly overcomplicating things by passing the DAO to the Cache instead of capturing it in a lambda?
Anything else I might have missed?

Cache semantic

A cache is something optional. You should be able to disable the cache by always returning "null" in the lookup method without worrying that your algorithm fails. It only should be slower. I don't think your implementation will pass this check. The cache should NOT retreive new data on its own.

Cache scope

A cache has a well defined lookup and object scope. Your implementation has a cached object lookup scope of "method" and a cached object usage scope that is somehow greater. Both should fall into the same scope to satisfy symmetry. Without knowing your UseCase-Layer you most of the time want to have all objects needed available in the UseCase. Refreshing your UseCase will often cause refreshing your cache because you want to process a UseCase on the latest data.

One other thing: Your Child-Objects may not the only Objects that have references on the Parent-Object. Those objects you would have to handle with seperate caches so you cannot take advantage of the already loaded instances. Consider caching on In thhe Business-Layer if you have no Persistence Mapping Tool (JPA). Otherwise you should make use of Entity-Caches.

Lambda expressions

My opinion on lambda expressions is ambivalent. Of course you can have very compact and technically expressive statements. But technically expressive statements may not be semantically expressive. Often they hide bad design. You can compress code that it looks good in one method that would normally be splitted into two methods. But the only reason you may have need for two methods was bad design.

I consider usage of lambda expression rarely: Listener-notifications, mapping JPA objects to Busines-Objects or massive parallel number calculations. I never use lambda expressions to make code "shorter".

Approach

Simplify the data retreiving process:

public class ChildService {

    public List<Child> getChildren() {

        List<Child> result = new ArrayList<>();

        try (ResultSet resultSet = connection.createStatement().executeQuery("")) {

            while (resultSet.next()) {
                result.add(new ChildProxy(resultSet.getLong("parent_id")));
            }

        } catch (SQLException e) {
            ...
        }

        return result;
    }

}

Passing the parent id to the constructor of the Child object and resolving the parent through the DAO:

public class Child {

    protected long parentId;

    public Child(long parentId) {
        this.parentId = parentId;
    }

    public Parent getParent() {
        return new ParentDaoImpl().getById(this.parentId);
    }

}

Derive a Caching enabled ChildProxy from the "Child" and intercept the getParent()-method to check for a cached Parent-Object. If you would have a JPA-Layer you may not need this because OR-Mapper often have their own caching mechanisms that are sufficient for most usecases.

public class ChildProxy extends Child {

    public ChildProxy(long parentId) {
        super(parentId);
    }

    Parent getParent() {

        Parent parent = Cache.lookup(Parent.class, this.parentId);

        if (parent == null) {
            parent = super.getParent();
            Cache.put(this.parentId, parent);
        }

        return parent;
    }

}

Formulating your UseCase:

public class UseCase {

    private final List<Child> children = new ArrayList<>();

    public void init() {
        ...
        this.children.addAll(new ChildService().getChildren());
        ...
    }

    public void reset() {
        Cache.clear();
        this.children.clear();
        init();
    }

    ...

}

Maybe you have to think a little bit more about the caching scope. It's not that easy because most developers tend to simply store expensive objects near their first occurance.

My advice is: NEVER cache at the first place. ALWAYS formulate your statements as they are NOT EXPENSIVE. After that you can go on with an optimization...

... that should be transparent to the existing implementation

... and has a well defined scope to match the use case requirements


Find a Id that contains all mentioned values I have a scenario to find the CategoryId which all have the mentioned Mode values.

The schema of the table ModeTester:

CREATE TABLE ModeTester (
    ModeId      INT IDENTITY(1, 1) NOT NULL,
    CategoryId  INT,
    Mode        CHAR(1),
    ModeValue   VARCHAR (20)
)

The dummy values:

INSERT INTO ModeTester (CategoryId, Mode, ModeValue)
SELECT 1, 'H', 'Hospital' UNION
SELECT 2, 'W', 'Work' UNION
SELECT 1, 'R', 'Rating' UNION
SELECT 3, 'W', 'Window' UNION
SELECT 3, 'H', 'Home' UNION
SELECT 3, 'R', 'Rail' UNION
SELECT 4, 'R', 'Royal' UNION
SELECT 4, 'H', 'Hide' UNION
SELECT 4, 'W', 'Work'

From the above data, I want to get the result of CategoryId those should have all Mode values W, R, H.

So, I wrote this query

SELECT C.CategoryId
FROM (
    SELECT CategoryId, Count(CategoryId) [CategoryCount]
    FROM ModeTester
    WHERE Mode IN ('H', 'W', 'R')
    GROUP BY CategoryId
) C
WHERE C.CategoryCount = 3

It returns the expected data:

CategoryId
----------
 3
 4

But, I want to know if there are any better ways to achieve this or fine-tune the query.

Here's a sample SQL Fiddle.

Hard-coding the CategoryCount = 3 is not helped on all cases. If the data increases, checking on category count condition is not working.

You can use a HAVING clause to filter your GROUP BY result.

SELECT CategoryId
FROM ModeTester
WHERE Mode IN ('H', 'W', 'R')
GROUP BY CategoryId
HAVING Count(CategoryId) = 3


Simple Game Engine in JavaScript I’m planning on creating a Mortal Kombat-inspired game in JavaScript. I’ve written a basic game engine that manages my game states and creates a game loop.

I just wondered if it could be improved at all? As I’m a web developer by trade and this is my first foray into programming games.

/**
 * Game engine.
 */
function GameEngine() {
    this.states = new Array();
    this.currentState;
    this.running = false;
};

GameEngine.prototype.init = function() {
    this.running = true;
};

GameEngine.prototype.changeState = function(state) {
    this.currentState = state;
};

GameEngine.prototype.isRunning = function() {
    return this.running;
};

GameEngine.prototype.handleEvents = function() {
    this.currentState.handleEvents(this);
};

GameEngine.prototype.update = function() {
    this.currentState.update(this);
};

GameEngine.prototype.draw = function() {
    this.currentState.draw(this);
};

/**
 * Base game state state.
 */
function GameState() {};

/**
 * Intro state.
 */
function IntroState() {};

IntroState.prototype = Object.create(GameState.prototype);

IntroState.prototype.handleEvents = function(game) {
    console.log('IntroState::handleEvents');
};

IntroState.prototype.update = function(game) {
    console.log('IntroState::update');
};

IntroState.prototype.draw = function(game) {
    console.log('IntroState::draw');
};

/**
 * Initialises the game.
 */
(function() {
    var game = new GameEngine();
    var gameLoop;
    var frameLength = 500;

    game.init();
    game.changeState(new IntroState());

    gameLoop = setInterval(function() {
        game.handleEvents();
        game.update();
        game.draw();
    }, frameLength);
})();

There's really not much to say about this code, so I'll quote a comment posted on this question:

  [...] a basic game engine is a loop and a redraw/update system. So yes you are good. You should explore other game engines in javascript and see what they do. – Steven

Just one thing that's itching: you're declaring var frameLength = 500; - I think this is going to bite you later, when you want to adjust frame rate (FPS).


Largest palindrome in a string I am working on a question in which I need to find a largest palindrome given a string. Here is the description.

public class LargestPalindrome {
  public static void main(String[] args) {
    System.out.println("Longest Palindrome 1: " +findLongestPalindrome("xyzracecar"));
    System.out.println("Longest Palindrome 2: " +findLongestPalindrome("abcexcbaddd"));
    System.out.println("Longest Palindrome 3: " +findLongestPalindrome("cacxx"));
  }

  public static String findLongestPalindrome(String str) {
    String palindrome = ""; // initializing empty string

    for(int i=0; i<str.length(); i++) {
      for(int j = str.length(); j>i; j--) {
       String data = str.substring(i, j);
        if(isPalindrome(data)) {
          if(data.length() > palindrome.length()) {
            palindrome = data;
          }
        }
      }
    }

    return palindrome;
  }

  /**
   * This method is used to check whether a given string is palindrome or not
   *
   */
  public static boolean isPalindrome(String s) {
    int last = s.length() - 1;

    for(int i=0; i<s.length()/2; i++) {
      if(s.charAt(i) != s.charAt(last)) {
        return false;
      }
      last--;
    }

    return true;
  }
}

The code is working fine. Can it be optimized anymore?

Two minor points on the code and one on unit test:

Combining if clauses

You can combine both if clauses as such:

if (isPalindrome(current) && current.length() > palindrome.length()) { ... }

Enhancing use of for loop

Multiple local variables can be declared in the for loop, so you can include your last variable as such:

private static boolean isPalindrome(String s) {
    for (int i = 0, last = s.length() - 1; i < s.length() / 2; i++, last--) {
        if (s.charAt(i) != s.charAt(last)) {
            return false;
        }
    }
    return true;
}

Unit testing

As mentioned in my comment, it will be better to turn your main() code into a series of unit tests to ensure your logic is working correctly. The example below uses TestNG and Hamcrest matchers, and is meant to be a simple showcase for how this can be done. You should consider a variety of tests (e.g. empty Strings, non-palindromic inputs) to also ensure it works for as many cases as possible. That is when you can consider some of the features of TestNG such as parameterized testing.

@Test
public void doTest() {
    assertThat(findLongestPalindrome("xyzracecar"), equalTo("racecar"));
    assertThat(findLongestPalindrome("abcexcbaddd"), equalTo("ddd"));
    assertThat(findLongestPalindrome("cacxx"), equalTo("cac"));
}

Bonus: Oh yeah, is it the largest or longest palindrome you are looking for? Since you are dealing with Strings, my suggestion is to be consistent and stick with permutations of 'longest palindrome'. :)


A reusable Previous/Next Contact Collection ViewModel class I often use a ViewModel class that contains a collection of Contacts and allows me to iterated through the collection with Previous and Next buttons.

I find that I've got duplicate versions of this class all over the place and I'd like to make my code reusable.

How would I go about making the following code reusable (i've been going round in circles for a while now):

public class PartiesViewModel : BaseViewModel
{
    const int MaxParties = 20;

    public ObservableCollection<Party> Parties { get; set; }

    public PartiesViewModel(ObservableCollection<Party> parties)
    {
        Parties = parties;
        Party = Parties.FirstOrCreateIfEmpty();

        Party.PropertyChanged += UpdatePartyShortNamesList;
        Parties.CollectionChanged += Parties_CollectionChanged;

        ClearCommand = new RelayCommand(ClearPressed, CanClear);
        NextCommand = new RelayCommand(NextPressed, CanPressNext);
        PrevCommand = new RelayCommand(PrevPressed, CanPressPrev);
    }

    void UpdatePartyShortNamesList(object sender, System.ComponentModel.PropertyChangedEventArgs e)
    {
        if (e.PropertyName == "ShortName")
            NotifyPropertyChanged("PartyShortNames");
    }
    void Parties_CollectionChanged(object sender, NotifyCollectionChangedEventArgs e)
    {
        NotifyPropertyChanged("PartyNames");
        NotifyPropertyChanged("PartyShortNames");
        NotifyPropertyChanged("Count");
    }

    Party _Party;
    public Party Party
    {
        get { return _Party; }
        set
        {
            _Party = value;
            NotifyPropertyChanged("Party");
            NotifyPropertyChanged("Count");
        }
    }

    public string Count
    {
        get
        {
            var list = Parties.ToList();
            list.RemoveAll(x => x.IsEmpty());
            int current = list.FindIndex(x => x == Party) + 1;
            int count = list.Count();
            if (current == 0)
                return string.Format("{0} of {0}", count + 1);
            else
                return string.Format("{0} of {1}", current, count);
        }
    }

    public RelayCommand PrevCommand { get; set; }
    public bool CanPressPrev() { return IndexOf(Party) > 0; }
    public void PrevPressed()
    {
        try
        {
            int i = IndexOf(Party);
            Party = Parties[i - 1];
        }
        catch (Exception ex) { Message.ShowError(ex); }
    }

    public RelayCommand NextCommand { get; set; }
    public bool CanPressNext() { return !Party.IsNullOrEmpty() && IndexOf(Party) != MaxParties; }
    public void NextPressed()
    {
        try
        {
            if (IndexOf(Party) + 1 == Parties.Count)
            {
                Party = new Party();
                Parties.Add(Party);
            }
            else
            {
                int i = IndexOf(Party);
                Party = Parties[i + 1];
            }
        }
        catch (Exception ex) { Message.ShowError(ex); }
    }

    public RelayCommand ClearCommand { get; set; }
    public bool CanClear()
    {
        return !Party.IsNullOrEmpty();
    }
    public void ClearPressed()
    {
        try
        {
            int i = IndexOf(Party);
            if (Party == Parties.Last())
                Party.Clear();
            else
            {
                Parties.RemoveAll(x => x == Party);
                Party = Parties[i];
            }
        }
        catch (Exception ex) { Message.ShowError(ex); }
    }

    int IndexOf(Party party) { return Parties.ToList().FindIndex(x => x == party); }

    public static List<string> PartyTypes { get { return new List<string>() { "Individual", "Company", "Partnership" }; } }

    public bool IsCompany { get; set; }
}

A couple of comments:

It looks like the class only manages parties. This is nice SRP.
Use StyleCop, it makes code much easier to read and review.
Validate inputs, makes it much faster to find and fix bugs:

public PartiesViewModel(ObservableCollection<Party> parties)
{
    if (parties == null)
    {
        throw new ArgumentNullException(nameof(parties));
    }
    ...

Party = Parties.FirstOrCreateIfEmpty(); without knowing what this extension method does I don't love it as it requires me to navigate to another place in the code to see what it does. I guess it does something like this:

Party = Parties.FirstOrDefault() ?? new Party(...);

If this is the case I think you should drop the extension method.
Party.PropertyChanged += UpdatePartyShortNamesList;

I can't see that this subscription is updated anywhere. I would expect the subscription to be removed from the old party and added to the new party in the setter for Party. As it is now it is probably a bug and a memory leak.
public ObservableCollection<Party> Parties { get; set; } a public setter here suggests that the collection can be replaced (without notifying the view about it). I assume you want it to be a readonly property. This also applies to the command properties.

In C#6 you express this as:

public ObservableCollection<Party> Parties { get; }

Prior to C#6 this was slightly more verbose:

private readonly ObservableCollection<Party> _parties;

public PartiesViewModel(ObservableCollection<Party> parties)
{
    _parties = parties;
}

public ObservableCollection<Party> Parties
{
    get { return _parties; }
}

Being explicit about what is readonly is a very good habit as it makes code easier to reason about and lets the compiler catch more bugs.
I have a feeling that binding SelectedIndex can simplify things a lot.
The command methods CanPressPrev etc. shall probably be private as the commands are normally the exposed means to interact with the viewmodel.
About reuse, this class looks like a nice candidate for composing with. Inject it in other viewmodels where the contact list is needed.

This got long, ending here.


Display progress bar to show async request status using jQuery Instead of using the ProgressBar plugin, this script displays the progress bar for async requests on the page. Could anyone provide any feedback on this, especially if there will be any issues like cross browser compatibility, etc?

       Loading...

//displays progress bar
$('.overlay').ajaxStart(function () {
       $(this).css({ height: $(document).height(), width: $(document).width() }).show();
       $(this).find(".progress").css({ top: $(window).height() / 2, left: $(window).width() / 2 });
       }).ajaxStop(function () {
       $(this).hide();
});

.overlay
{
    position: fixed !important;
    position: absolute; /*ie6*/
    width: 100%;
    top: 0px;
    left: 0px;
    right: 0px;
    bottom: 0px;
    background-color: #000;
    filter: alpha(opacity=20);
    opacity: 0.2;
    -moz-opacity: 0.2;
    -khtml-opacity: 0.2;
    -webkit-opacity: 0.2;
    z-index: 10004;
    display: none;
    filter: progid:DXImageTransform.Microsoft.Alpha(opacity=20); /*ie6*/
}
.overlay .progress
{
    position: absolute;
    z-index: 10005;
    background: #fff;
    color: #000;
}

Realizing that you are trying to make reusable code, I figured I would share my methodology, as I believe it will extend your structure here.

I like to wrap everything into an object with functions appropriate to AJAX, and I added in your .overlay components:

var ajax = {
        showProgressBar:function($overlay){
            var $doc = $(document),
                $win = $(window);

            $overlay
                .css({
                    height:$doc.height(),
                    width:$doc.width(),
                    display:'block'
                })
                .find('.progress')
                .css({
                    top:($win.height() / 2),
                    left:($win.width() / 2)
                });
        },
        hideProgressBar:function($overlay){
            $overlay.hide();
        },
        abortActive:function(){
            var self = this;

            if(($.active > 0) && (self.active !== undefined)){
                self.active.abort();
            }
        },
        clearActive:function(){
            this.active = undefined;
        },
        send:function($type,$url,$data,$success,$error,$abort){
            var self = this,
                $overlay = $('.overlay'),
                $beforeSend = ($abort ? function(){
                    self.abortActive();
                    self.showProgressBar($overlay);
                } : function(){
                    self.showProgressBar($overlay);
                }),
                $complete = ($abort ? function(){
                    self.clearActive();
                    self.hideProgressBar($overlay);
                } : function(){
                    self.hideProgressBar($overlay);
                }),
                curFunc = $.ajax({
                    type:$type,
                    url:$url,
                    data:$data,
                    beforeSend:$beforeSend
                }).success($success).error($error).complete($complete);

            this.active = ($abort ? curFunc : self.active);
        }
    };

And then it is called with an example of something like this:

ajax.send(
    'POST',
    'requires/submitSomething.php',
    {
        id:$someId,
        type:$someType
    },
    function(data){
        alert('Successful! The response message was: '+data.message);
    },
    function(data){
        alert('Error! The response message was: '+data.message);
    },
    false
);

This is obviously super generic, but should do the trick. A quick explanation:

the beforeSend setting will fire a callback function prior to sending the AJAX, which is where you want to load your progress gif
the complete setting will fire a callback function once everything else completes, including the success / error functions, so it is where you want to hide the progress gif
in the success / error functions, you can include the response data and manipulate as you wish
the use of this in the send function refers to the object parent, hence able to see the other functions rather than referring to them by name
the ajax call is assigned to a variable to allow for aborting the call mid-stream before starting another (if another action overrides it, for example) ... the reason for the use of this rather than self is because we need to assign it to the object rather than just reference the existing objects within it
the function creation is based on the boolean you pass in for $abort, to determine if your AJAX call should stop all current AJAX calls or have it run in parallel (depending on your usage)

And a few small tweaks I made to your existing jQuery code to optimize:

cached $(document), $(window), and $('.overlay') as they are used more than once in the functions
consolidated your .show() function into the previous CSS statement (as the default is to basically perform a .css({display:'block'}) statement ... if you have overridden this with a custom function, you can return as needed)
chained the calls to $('.overlay') and $('.progress'), as they were both based on $(this)

This function should give you a good starting point, and you can modify as needed if you need additional parameters. The only additional parameter I advise against is the use of dataType; for some reason, everyone using json likes to include dataType in their AJAX call (which is an intelligent guess, not a true marker) instead of doing it right. Don't be "that guy", put your appropriate header declaration in the files you are loading via AJAX and it will be bulletproof.

Hope this helped.


Find no of swaps needed to club identical elements of array together I have created a solution for the problem as below:

  Given a string S consisting of letters 'O' and 'Z'. Calculate the
  minimum number of changes required such that all O's are adjacent to
  each other and all Z's are adjacent to each other. One change is
  equivalent to swapping two neighbouring letters

  for e.g for a given array: oozozo  desired array is : oooozz so,min no
  of swap would be 3,which should be the desired output.

Let me know if the solution has any bad practices.

My Code:

package com.test.practice;

import java.io.BufferedReader;
import java.io.InputStreamReader;

public class TestClass {

    public static void main(String args[]) throws Exception {

        BufferedReader br = new BufferedReader(new InputStreamReader(System.in));
        String numberOfInputs = br.readLine();

        for (int i = 0; i < numberOfInputs.length(); i++) {
            String inputString = br.readLine();

            findNumberOfTurns(inputString);
        }

    }

    private static void findNumberOfTurns(String inputString) {

        int count = 0;
        char[] input = inputString.toCharArray();
        char firstCharacter = input[0];
        char secondCharacter;
        boolean secondCharEncountered = false;
        int countSecondChar = 0;
        for (int i = 1; i < input.length; i++) {
            if (firstCharacter == input[i]) {
                if (!secondCharEncountered) {
                    countSecondChar=0;
                } else {
                    count+=countSecondChar;
                    secondCharEncountered=false;
                }
            } else {
                secondCharEncountered=true;
                countSecondChar+=1;
            }
        }

        System.out.println(count);
    }

}

Bug

Your method doesn't calculate the correct number of swaps. Using the input string oozzoo the correct result should be 4 but your algorithm gets a result of 2.

Single resopsonsibility principle

The findNumberOfTurns() method is calculating the number of swaps and prints the output which is too much. Let the method return the number of swaps needed. Printing the result shouldn't be in that method.


Bilinear resizing algorithm I have the next bilinear resizing algorithm, and even though I use fixed point is still quite slow for my requirements. In the past I tried to cache some values in tables but the improvement was of a few not noticeable milliseconds.

Is any other way I can improve the speed? As this is intended for ARM processors, I know I could write it with NEON instructions, but unfortunately I don't have the knowledge for such task.

void resize_bilinear(const unsigned int *input, unsigned int *output, const unsigned int sourceWidth, const unsigned int sourceHeight, const unsigned int targetWidth, const unsigned int targetHeight)
{
    unsigned int widthCoefficient, heightCoefficient, x, y;
    unsigned int pixel1, pixel2, pixel3, pixel4;
    unsigned int hc1, hc2, wc1, wc2, offsetX, offsetY;
    unsigned int r, g, b, a;

    const unsigned int wStepFixed16b = ((sourceWidth - 1) << 16)  / ((unsigned int)(targetWidth - 1));
    const unsigned int hStepFixed16b = ((sourceHeight - 1) << 16) / ((unsigned int)(targetHeight - 1));

    heightCoefficient = 0;

    int offsetPixelY;
    int offsetPixelY1;
    int offsetX1;

    for (y = 0; y < targetHeight; y++)
    {
        offsetY = (heightCoefficient >> 16);
        hc2 = (heightCoefficient >> 9) & (unsigned char)127;
        hc1 = 128 - hc2;

        widthCoefficient = 0;

        offsetPixelY  = offsetY * sourceWidth;
        offsetPixelY1 = (offsetY + 1) * sourceWidth;

        for (x = 0; x < targetWidth; x++)
        {
            offsetX = (widthCoefficient >> 16);
            wc2 = (widthCoefficient >> 9) & (unsigned char)127;
            wc1 = 128 - wc2;

            offsetX1 = offsetX + 1;

            pixel1 = *(input + (offsetPixelY  + offsetX));
            pixel2 = *(input + (offsetPixelY1 + offsetX));
            pixel3 = *(input + (offsetPixelY  + offsetX1));
            pixel4 = *(input + (offsetPixelY1 + offsetX1));

            a = ((((pixel1 >> 24) & 0xff) * hc1 + ((pixel2 >> 24) & 0xff) * hc2) * wc1 +
                (((pixel3  >> 24) & 0xff) * hc1 + ((pixel4 >> 24) & 0xff) * hc2) * wc2) >> 14;

            r = ((((pixel1 >> 16) & 0xff) * hc1 + ((pixel2 >> 16) & 0xff) * hc2) * wc1 +
                (((pixel3  >> 16) & 0xff) * hc1 + ((pixel4 >> 16) & 0xff) * hc2) * wc2) >> 14;

            g = ((((pixel1 >> 8) & 0xff) * hc1 + ((pixel2 >> 8) & 0xff) * hc2) * wc1 +
                (((pixel3  >> 8) & 0xff) * hc1 + ((pixel4 >> 8) & 0xff) * hc2) * wc2) >> 14;

            b = ((((pixel1) & 0xff) * hc1 + ((pixel2) & 0xff) * hc2) * wc1 +
                (((pixel3)  & 0xff) * hc1 + ((pixel4) & 0xff) * hc2) * wc2) >> 14;

            *output++ = (a << 24) | (r << 16) | (g << 8) | b;

            widthCoefficient += wStepFixed16b;
        }

        heightCoefficient += hStepFixed16b;
    }
}

You could be saving important variables that are being used frequently in the register like the following:

register const unsigned int targetWidth

Make sure memory accesses are cache-optimized, i.e. clumped together.

Investigate if signed/unsigned for integer operations have performance costs on your platform.

Investigate if look-up tables rather than computations gain you anything (but these can blow the caches, so be careful).

Even though you've already avoided floats I'm going to post this anyway:
If you can avoid double and float variables, use int. On most architectures, int would be test faster type for computations because of the memory model. You can still achieve decent precision by simply shifting your units like you've been doing (ie use 1026 as int instead of 1.026 as double or float).

And, of course, do lots of profiling and measurements.


Abstracting the process of finding the index of the maximum in Python Suppose I want to find the index of the maximal value of a given sequence. In Matlab, this is:

[value, index] = max(x)

In Python, I would abstract the following pattern:

def find_best_element_index(elements, is_better):
    best_so_far = elements[0]
    best_so_far_index = 0
    for (index, element) in enumerate(elements):
        if is_better(element, best_so_far):
            best_so_far = element
            best_so_far_index = index
    return best_so_far_index

Which I could use in the following way:

import operator
assert find_best_element_index([6, 3, 9, 8, 0, 9, 7, 9, 7, 8], operator.gt) == 2

But I wonder if there is a more pythonic way to go when I just want to find the index of the "best" item.

I just want to find the index of the "best" item.

This can be written very shortly and readably using enumerate, first class functions and list indexing.

def index_of_max_value(items):
    return max(enumerate(items), key = lambda x: x[1])[0]

Explanation:

I create a list that contains the numbers and their indexes, the index is first, the number second.

>>> list(enumerate([4,7,4,8]))
[(0, 4), (1, 7), (2, 4), (3, 8)]

I ask the item that has the biggest value when a function I decide is applied to it, I do so by passing the function as the key argument, when functions are passed around like this they are first class citizens

>>> max(["rg","gu","sd"], key = lambda s: sum(map(ord,s)))
'gu'

For example in the above I asked which of the strings had the most ASCII value.

key = lambda x: x[1]

Means that I want the pair (index,number) that has the biggest 'second' item (remember zero indexing).

[n] gives you the n-th item of a list, I ask the '0-th' (first) item that is the index.

I hope that you like my alternative shorter solution, if you have any doubts, fell free to ask.


Calculation of next week number (constrained by max 3) with rollover What I want my code to do is that if we're on day 3 then we want to increase the week by 1. But if week number is 3 we want to reset the week to 1. The code below works but can it be simplified?

Java code

public int calculateNextWeek(int week, int day) {
    if (day == 3) {
        if (week == 3) {
            return 1;
        } else {
            return week + 1;
        }
    }

    return week;
}

Example data and expected output:

day = 3, week = 1 => 2
day = 3, week = 3 => 1
day = 1, week = 1 => 1
day = 1, week = 3 => 3

You can use the modulo operator to wrap around your week counter:

public int calculateNextWeek(int week, int day) {
    if (day == 3) {
        return (week % 3) + 1;
    }

    return week;
}


"Reflecting" addition I'm implementing the following function for signed integer numbers: given endpoints of a closed interval lo and hi, and inputs x (in [lo..hi]) and dx, I'd like to compute the "reflected sum":
if x + dx is still in [lo..hi] then that's it; otherwise the underhanging or overhanging portion is "folded back" onto the result (shown as y below). Basically, x is "bouncing around" in the interval.

                             overhang
                             /--\
                     dx
                 -------------->
--------[-------|-------|---]---|------
       lo       x       y   hi  x + dx
                        <---

Because this is for a hardware circuit, I am doing this using Clash's sized Signed integer type and width-extending addition, otherwise it is normal Haskell code.

My current implementation is the following:

bounce :: (KnownNat n, KnownNat k, (k + 1) <= n) => (Signed n, Signed n) -> Signed n -> Signed k -> (Signed n, Bool)
bounce (lo, hi) x dx
    | under > 0 = (lo + under, True)
    | over > 0 = (hi - over, True)
    | otherwise = (resize new, False)
  where
    new = add x dx
    under = resize $ resize lo - new
    over = resize $ new - resize hi

Things I don't like about this implementation:

There is a lot of noise about bit widths (all the resize calls)
Overall, it is not immediately obvious by just looking at the code what it does. It feels like it's doing a lot of seemingly ad-hoc arithmetic operations that "just happen" to work.

I am looking for improvements that come from the structure of the problem, not "oh you should rename x to theNumberThatIsBouncingAround because one-letter variable names bad".

I don't think your code agrees with your spec: Your code looks like it can only implement a function built from three line segments.

You shouldn't need explicit resize.

bounce (lo, hi) x dx = let
  (bounces, remainder) = divMod (add dx $ x-lo) $ hi-lo
  in (if even bounces then lo + remainder else hi - remainder
     ,bounces /= 0)


Validating password rules The following method is designed to return true if it passes all of the rules for a password. Does anyone see a way to improve it? Performance improvements are welcome for sake of education.

I already know password rules beside length are counter-productive.  It's not my choice.  Also, I know that performance is absolutely trivial here.  I only care about performance by way of educating myself.  The stuff I learn on non-critical optimization often helps me when I do have a bottleneck.  I will not be implementing something just because it is faster.

/**
 * Returns true if and only if a password passes the rules:
 *  - Must be at least 8 characters.
 *  - Must contain two of the following types:
 *    - Letters
 *    - Numbers
 *    - Symbols (includes whitespace)
 *
 * @param string $password The raw, unencrypted password.
 * @return bool
 */
public function isValidPassword($password) {
    $length = strlen($password);

    if ($length < 8) {
        return false;
    }

    $foundLetter = false;
    $foundNumber = false;
    $foundSymbol = false;

    for ($i = 0; $i < $length; $i++) {
        $char = $length[$i];

        if (ctype_alpha($char)) {
            $foundLetter = true;
        }
        else if (ctype_digit($char)) {
            $foundNumber = true;
        }
        else if (ctype_punct($char) || ctype_space($char)) {
            $foundSymbol = true;
        }
    }

    return ($foundLetter && $foundNumber)
        || ($foundLetter && $foundSymbol)
        || ($foundNumber && $foundSymbol);
}

Your solution does not honor multibyte characters, i.e. it will probably break for non-English languages.

Instead you could utilize PCRE (regular expressions) and Unicode character grounps:

$foundLetter = preg_match('(\pL)u', $password);
$foundNumber = preg_match('(\pN)u', $password);
$foundSymbol = preg_match('(???)u', $password);

Not sure what you define as a symbol, [\pP\p{Xps}] (Punctuation + Whitespace) would reflect your current version (I think). Maybe a better alternative would be [^\pL\pN] though, which just ensures one non alphanumeric letter.

In theory you could change your script into a one liner now:

return strlen($password) >= 8 && (
    preg_match('(\pL)u', $password)
  + preg_match('(\pN)u', $password)
  + preg_match('([^\pL\pN])u', $password)
) >= 2;


Find the length of the longest Common Subsequence I have a DP based implementation for the Longest Common Subsequence(LCS) problem
to find the length of the LCS.

I wanted to know if there is room for improvement in terms of efficiency(space/time).

public int longestCommonSubsequence(char[] firstWord, char[] secondWord) {
        int[][] lcsMatrix = new int[firstWord.length+1][secondWord.length+1];

        for(int i = 0; i < firstWord.length; i++) {
            for(int j = 0; j < secondWord.length ; j++) {

                if (i == 0 || j == 0) {
                    lcsMatrix[i][j] = 0;
                }
                else if(firstWord[i] == secondWord[j]){
                   lcsMatrix[i][j] = lcsMatrix[i-1][j-1]+1;
                }
                else{
                    lcsMatrix[i][j] = Math.max(lcsMatrix[i][j-1],lcsMatrix[i-1][j]);
                }
            }

        }
        return lcsMatrix[firstWord.length-1][secondWord.length-1];
    }

Correctness:

It seems your implementations is incorrect since you are always ignoring the first character.

Here in both loops the range should be closed:

  for(int i = 0; i <= firstWord.length; i++) {
       for(int j = 0; j <= secondWord.length ; j++) {
           ...
       }
   }

And the second if inside the inner loop should look like this:

else if(firstWord[i - 1] == secondWord[j - 1]) {
    ...
}

finally the function should return this:

 return lcsMatrix[firstWord.length][secondWord.length];

DRY your code:

Since in java an array of primitive integers are initialized to zero there is no need for the first if, therefore you can start the loop at i = 1 and j = 1. You could also use a ternary operator after getting rid of the if statement(although some people may prefer the if else).

Final code

    public static int longestCommonSubsequence(char[] firstWord, char[] secondWord) {
        int[][] lcsMatrix = new int[firstWord.length + 1][secondWord.length + 1];
        for(int i = 1; i <= firstWord.length; i++) {
            for(int j = 1; j <= secondWord.length ; j++) {
                lcsMatrix[i][j] = (firstWord[i - 1] == secondWord[j - 1])
                        ? lcsMatrix[i - 1][j - 1] + 1
                        : Math.max(lcsMatrix[i][j - 1],lcsMatrix[i - 1][j]);
            }
        }
        return lcsMatrix[firstWord.length][secondWord.length];
    }


Get part of element ID I am getting the number part of the container ID that the button is in. Now I am wondering if I need to create an array using split and then put this number in a variable, or if there is a more efficient way to get it.

.match() also returns an array, so this is almost the same as using split.

I mean the code works and it's ok, but I'm curious to see if there is a better way.

 $('.add').on('click', function(e) {
   e.preventDefault();
   var $this = $(this);
   var tab_number_array = $this.parents('.tab_content').attr('id').split('_');
   var tab_number = tab_number_array[1];
   alert(tab_number);
 });

      <button class="add">Add</button>

      <button class="add">Add</button>

      <button class="add">Add</button>

The parents method will traverse the DOM all the way up the chain. Rather use the closest method, which will stop at the first match:

$('.add').on('click', function(e) {
    e.preventDefault();

    var number = $(this).closest('.tab_content').attr('id').split('_')[1];

    alert(number);
});


Listing human-readable enums I'm trying to do some util class to operate on enums - convert Enums to its special strings representation via interface method.
String stored in enum constructors.

public enum StatusEnum implements HumanReadableEnum {
    NOT_VERIFIED("Not verified message"),
    INVALID("Invalid message"),
    APPROVED("Approved message");

    private String text;
    public String getHumanReadable(){   // single method of HumanReadableEnum
        return text;
    }

For example instead of HumanReadableEnum[] v = StatusEnum.values()
 I want to get List of inner text messages and do it in generic style.

So now I made a toHumanReadableCollection method:

public class EnumUtils {
    public static <CustomEnum extends Enum & HumanReadableEnum, E extends Class<CustomEnum>>
    List<String> toHumanReadableCollection(E enumClass){
        Enum[] values = enumClass.getEnumConstants();
        List<String> humanReadableCollection = new ArrayList<>(values.length);

        for (Enum value : values) {
            String stringRepresentation = ((CustomEnum)value).getHumanReadable();
            humanReadableCollection.add(stringRepresentation);
        }
        return humanReadableCollection;
    }
}

As a result I can do something like:

List<String> messagesOfEnum1 = EnumUtils.toHumanReadableCollection(MyEnum1.class);
List<String> messagesOfEnum2 = EnumUtils.toHumanReadableCollection(MyEnum2.class);

And have code of toHumanReadableCollection in single place. But I have some doubts about performance.
So could somebody help me to understand the cost of my code? :) Did I touched the reflection?)

Method header

Your method header can be improved, the generic type E is not necessary, it can instead be written as:

public static <CustomEnum extends Enum & HumanReadableEnum>
    List<String> toHumanReadableCollection(Class<CustomEnum> enumClass){

Type parameters should generally be one-character names, so this would be better as:

public static <E extends Enum & HumanReadableEnum>
    List<String> toHumanReadableCollection(Class<E> enumClass) {

Compiler warning

You're getting a compiler warning here:

String stringRepresentation = ((CustomEnum)value).getHumanReadable();

All you care about is the getHumanReadable method, which is in the HumanReadableEnum interface, so make that:

String stringRepresentation = ((HumanReadableEnum)value).getHumanReadable();

Naming and interface

HumanReadableEnum should instead be HumanReadable, there's no need to keep Enum in the name, there's no need to restrict the interface to only enums either.

Your questions

Reflection? Yes, this line of code is using reflection:

Enum[] values = enumClass.getEnumConstants();

Time complexity? \$O(n)\$, it is linear to the number of enum constants.

Java 8

In Java 8, you could replace your method with this:

List<String> list = Arrays.stream(StatusEnum.values())
   .map(HumanReadableEnum::getHumanReadable)
   .collect(Collectors.toList());

Or, as a general method, and also incorporating the earlier suggestions:

public static <E extends Enum & HumanReadable>
  List<String> toHumanReadableCollection(Class<E> enumClass) {
    Enum[] values = enumClass.getEnumConstants();
    return Arrays.stream(values)
       .map(e -> ((HumanReadable)e).getHumanReadable())
       .collect(Collectors.toList());
}

There's often a benefit of using Streams, I suggest you try to learn the Java 8 Stream API at some point.


Merge sort using c++ vectors I'm still learning C++ and also algorithms. So I'm expecting to do a lot of refactoring. Here's my code

#include <iostream>
#include <vector>
#include <iterator>

std::vector<int> mergesort(std::vector<int> &);
std::vector<int> merge(std::vector<int> &, std::vector<int> &);
std::vector<int>::iterator get_midpoint(std::vector<int> &);

int main()
{
  std::vector<int> vect;
  int num = 0;
  while(std::cin >> num)
  {
    vect.push_back(num);
  }
  std::vector<int> temp_vect = mergesort(vect);
  for(int num: temp_vect) {
    std::cout << num  << std::endl;
  }

}

std::vector<int> mergesort(std::vector<int> &unsorted_vector)
{
  std::vector<int>::iterator middle = unsorted_vector.begin();
  std::vector<int> sorted_vect, first_half, second_half, first_temp, second_temp;
  if(unsorted_vector.size() == 1) {
    return unsorted_vector;
  } else {
    middle = get_midpoint(unsorted_vector);
    first_temp.insert(first_temp.begin(), unsorted_vector.begin(), middle);
    second_temp.insert(second_temp.begin(), middle, unsorted_vector.end());
    first_half = mergesort(first_temp);
    second_half = mergesort(second_temp);
    sorted_vect = merge(first_half, second_half);
    return sorted_vect;
  }
}

std::vector<int> merge(std::vector<int> & first_vect, std::vector<int> & second_vect)
{
  std::vector<int> sorted_vect;
  auto first_it = first_vect.begin(),
        second_it = second_vect.begin();

  while(first_it != first_vect.end() && second_it != second_vect.end()) {
      if(*first_it < *second_it) {
        sorted_vect.push_back(*first_it);
        first_it++;
      } else {
        sorted_vect.push_back(*second_it);
        second_it++;
      }
  }

  sorted_vect.insert(sorted_vect.end(), first_it, first_vect.end());
  sorted_vect.insert(sorted_vect.end(), second_it, second_vect.end());

  return sorted_vect;
}

std::vector<int>::iterator get_midpoint(std::vector<int> &vect)
{
  std::vector<int>::iterator it = vect.begin();
  int middle = 0;
  if(vect.size() % 2 == 0) {
    middle =  vect.size() / 2;
  } else {
    middle = (vect.size() - 1) / 2;
  }
  std::advance(it, middle);
  return it;
}

I think you input routine is not really safe. maybe add an explicit break?

while (true) {
    cin >> num;
    if (cin.fail)
        break;
    vect.push_back(num);
}

What happens when unsorted_vector is empty. You check for size == 1 maybe turn that into size <= 1
Stuff like this is hard to read:

std::vector<int> sorted_vect, first_half, second_half, first_temp, second_temp;

I would definitely suggest to put every name onto its own line and add the explicit type. Its not that you are saving trees here.
Do not use else after an early exit.

if(unsorted_vector.size() == 1) {
    return unsorted_vector;
} else {
// Stuff
}

Is equivalent to:

if(unsorted_vector.size() == 1) {
    return unsorted_vector;
}
// Stuff

However with less indentation and easier control flow.
You should definitely reserve memory in your merge function:

vector<int> sorted_vect;
sorted_vect.reserve(first_vect.size()+second_vect.size());

Use descriptive names. This is purely subjective, but whenever I see vect and friends i cringe. What do you really save from ommitting "or"?


Modular exponentiation in C++ I wrote the following code for a contest and worked fine for me. Could I make it a bit faster?

#include<iostream>
using namespace std;

int power(int,int,int);

int main(int argc,char** argv){
    int base,exponent,mod;
    cin>>base>>exponent>>mod;
    cout<<power(base,exponent,mod)<<endl;
    return 0;
}

int power(int base,int exponent,int mod){
    if(mod==1)return 0;
    int ans=1;
    for(int i=0;i<exponent;i++){
        ans=(ans*base)%mod;
    }
    return ans;
}

The usual way is a little more complex, but a lot faster when the exponent is large. It looks something like this:
template <class T>
T mul_mod(T a, T b, T m) {
    if (m == 0) return a * b;

    T r = T();

    while (a > 0) {
        if (a & 1)
            if ((r += b) > m) r %= m;
        a >>= 1;
        if ((b <<= 1) > m) b %= m;
    }
    return r;
}

template <class T>
T pow_mod(T a, T n, T m) {
    T r = 1;

    while (n > 0) {
        if (n & 1)
            r = mul_mod(r, a, m);
        a = mul_mod(a, a, m);
        n >>= 1;
    }
    return r;
}

pow_mod right-shifts n (the exponent) each iteration through the loop, so the number of iterations is proportional to the number of bits in the number (where yours in the question is proportional to the exponent itself). In other words, yours is linear on the exponent's magnitude, and this is roughly logarithmic exponent's magnitude.
Actual code review
Since this is CodeReview, not (for example) Stack Overflow, let's also take a look at reviewing your code.
Variable Definitions
You've defined multiple variables in a single definition:
int base,exponent,mod;

Many people find it more readable to define one variable per definition:
int base;
int exponent;
int mod;

... or at least use a separate line for each variable:
int base,
    exponent,
    mod;

Naming
A function's name should reflect what it really does. Using power for modular exponentiation borders on misleading. I'd rather the name included modular or at least mod.
formatting
At least IMO, a little white space can help readability quite a bit. For one example, instead of:
int power(int base,int exponent,int mod)

...I'd rather see a space after each comma:
int power(int base, int exponent, int mod)

In addition, where there's flow control, the controlled statements should be indented, so this:
if(mod==1)return 0;

would come out like:
 if (mod==1)
     return 0;

return from main
When execution "flows" off the end of main, there's an implicit return 0;, so you can eliminate that line from your code (though some prefer to keep it).
Use of endl
I'd avoid using std::endl (ever). Most of the time you really just want to write out a new-line, in which case \n is entirely adequate. On the (relatively rare) occasion that you really also want to flush the stream as endl also does, you should do so explicitly.
using namespace std;
Pulling in the entirety of namespace std; like this is generally frowned upon. It's all right for other (more sensibly designed) namespaces, but std defines a huge amount of "stuff", most of which you really don't want directly visible.


Checking if an array is sorted I've come up with two ways to check to see if array is sorted and return true if it is.

public static boolean checkSortedness(int[] data)
{
    for(int i = 1; i < data.length; i++)
    {
        if(data[i-1] > data[i])
        {
            return false;
        }
    }
    return true;
}

I've also seen the i start at 0 and compare the next element instead of the previous. The other way which involves another variable is

public static boolean isSorted(int[] data)
{
     int previous = data[0];
     for(int i = 1; i < data.length; i++)
     {
         if(previous > data[i])
             return false;
         previous = data[i];
     }
     return true;
}

Which way is better and are there any alternatives?

What should the array parameter be named? I call it data but I've  considered arr. This class is meant to be fairly generic.

Three other answers, and none have picked up on the bug yet, so, here's the bug, in the second version:

The input array new int[]{4} is sorted, right? The first method will return true. The second method will return true, fine... but, what about the array new int[]{} (the empty array)? The first method will return true. The second method will throw ArrayIndexOutOfBoundsException

The two methods are not the same. The second is buggy.

To me, this makes the first method the clear winner.

Now, while you are reading this, also note that in Java, the { brace should, by convention, be at the end of a line, not on a new line. What you have in your code is C* style coding (C, C++, C#, etc.).

Also, use final when appropriate....

Your code should look like:

public static final boolean checkSortedness(final int[] data) {
    for(int i = 1; i < data.length; i++) {
        if(data[i-1] > data[i]) {
            return false;
        }
    }
    return true;
}


Automated Tweet of new inbound Enquiries in PHP7 (Laravel 5.2) Background.

A User can submit an enquiry via a form hosted on a clients website, these are pushed onto a queue, and then taken off the queue for processing, to process several different parts of the enquiry asynchronously a single event is fired, this event is listened for by several different listeners, each one with a unique and distinct set of functionality.

This is the complete listener I've written to post the inbound Enquiry to twitter.

<?php

namespace App\Listeners\Enquiry;

use App\Events\NewEnquiryEvent;
use App\Exceptions\DebuggableException;
use Illuminate\Queue\InteractsWithQueue;
use Illuminate\Contracts\Queue\ShouldQueue;
use App\Models\Stores\Enquiry;

class Tweet
{
    public function handle(NewEnquiryEvent $event)
    {
        Enquiry::find($event->body->id)->reject(function ($enquiry) {
            return !$enquiry->shouldTweet();
        })->map(function ($enquiry) {
            return $enquiry->sendToTwitter();
        });
    }
}

Code intent

The main problem with your code is that it does not give any reasons as to why it performs the way it does which makes understanding, reviewing or upgrading it difficult.

$event->body->id

What is a body id on the event? is it always present? I'm assuming its an int but I guess it could contain letters? Does $event_id adequately describe it?

Enquiry::find(...)

Ok here I don't know the return signature of ::find() I assume it finds N instances of Enquiry based on most framework behaviour, does it return an array or a object container? can I call it $enqueries?

Next is the reject() function which I assume is like array_filter based on the name I guess it works in the inverse, checks that return true will be rejected? I have vague prior knowledge on this thanks to mind share of the chosen framework but if I had no knowledge I would assume it's your own function which would mean I need to read the implementation

Next we've got a nameless function that performs: !$enquiry->shouldTweet() there's no type hinting so I don't know if reject() is expecting a boolean at this point in time I'll have to read all of the function, my main question is what is an $enquiry and what does it mean when it does not "shouldTweet()".

By chance you've used a boolean operator on the return so I can now confirm a boolean return feeding back to reject() can we remove all doubt with a type hint?

Can we array_filter on $enqueries instead of reject()? Then we can talk about shouldTweet() without using !.

Let's assume based on your implementation details thus far that there are multiple enquiry objects that are being filtered on whether they should tweet.

Is canTweet() usable here? shouldTweet() implies there might be mitigating circumstances to whether we can or cannot tweet.

The last call you have is a custom map() function, a map function will typically convert child array elements into a different child array element but your operation next suggests it does not aim to do that, I think it aims to create a side effect, that's more suitable for array_walk than array_map.

The semantics is important because the inbuilt functions tell you what happens in the loop without having to read the loop, particularly helpful when a loop is a non trivial amount of lines long.

I'm assuming that sendToTwitter() returns NULL because I've seen no evidence of its use, it is returned to the mapping operation but the result of that is now lost in the fluent object as far as we're concerned from this call point, a type hint could have hinted at the truth.

Code Reuse

The other problem your code has is that the anonymous functions which by themselves cannot be reused, if we upgrade them to full functions we give the reader some context through the function name and we are then able to call the code from multiple different locations.

The function name should help tell future maintainers how much scope the function should be responsible for and telegraph what the function is supposed to do in context.

Strong binding to your chosen framework

Your code is strongly bound to your framework which means it will be harder to extract out of the framework in the future and potentially brittle every time the framework upgrades or any of its components upgrade.

It's tempting to think Laravel will last forever but we're definitely due a new paradigm switch in the PHP world, if you're not sure ask Drupal 6/7 developers about the dangers of framework dependent code.

Fluent interface is needlessly obscuring

Laravel's fluent interface coming out of Enquery::find() makes it unnecessarily difficult to figure out what reject and map are operating on and its unclear where the result of map are in that object, is there a way to extract them? If so I guess they are an array of nulls now? I can't tell from this call point.

The main problem with the fluent interface is that it's robbed you of the opportunity to name the result of your intermediate operation, a name which shows code intention is worth way more than your implementation code in code review and code maintenance.

Overly generic method name

The method name handle will probably not stay adequate if you start tweeting anything else in your system or add any other methods to tweet.

Handle what? Why? When? What operation is done when something is "handled"? Maybe we can call it tweet_enquiry or a name that suggests that we tweet the enquiry based on NewEnquiryEvent you will know this answer better than I do, translate what you know about the context into your code.

Below I've created some code that telegraphs to the reader certain information that I've made assumptions about all through reading the code

<?php

namespace App\Listeners\Enquiry;

use App\Events\NewEnquiryEvent;
use App\Exceptions\DebuggableException;
use Illuminate\Queue\InteractsWithQueue;
use Illuminate\Contracts\Queue\ShouldQueue;
use App\Models\Stores\Enquiry;

class Tweet
{
    /**
     * On enquiry event send a enquiry tweet if enquiry should do.
     *
     * @parm Enquiry
     *   The enquiry object.
     */
    public function tweet_enquiry_on_event(NewEnquiryEvent $event) : void
    {
        $event_id = (int) $event->body->id;
        $enqueries = (array) Enquiry::find($event_id);
        $enqueries_that_should_tweet = array_filter($enqueries, 'enquiry_should_tweet');
        array_walk($enqueries_that_should_tweet, 'enquiry_send_to_twitter');
    }
}

// These functions are probably best off near the enquiry object.

/**
 * Should an enquiry object be able to tweet?
 *
 * @param Enquiry
 *   The enquiry object.
 *
 * @return bool
 *   Whether an enquiry should be able to tweet.
 */
function enquiry_should_tweet(Enquiry $enquiry) : bool
{
    $enquiry_should_tweet = $enquiry->shouldTweet();
    return $enquiry_should_tweet;
}

/**
 * Send the enquiry to twitter.
 *
 * @parm Enquiry
 *   The enquiry object.
 */
function enquiry_send_to_twitter(Enquiry $enquiry) : void
{
    $enquiry->sendToTwitter();
}