Check if two numbers are the same in a box I am making a Sudoku game and I have managed to add random numbers, add numbers, and to checklines (if there is 2 numbers on the same line). I am trying to check the 3x3 boxes if the number is twice or more in the same box. I have managed to do it one way, but is this the easiest way? In that case I need to do this 81 more times, or is there a better way?

 bool checkBox(){
int box1[3][3];

for (int i = 0; i <= 2; i++){
    for (int j = 0; j <= 2; j++){
        box1[i][j] = board[i][j];
    }
}

if (box1[0][0] != 0){
    if (box1[0][0] == box1[0][1] || box1[0][0] == box1[0][2] || box1[0][0] == box1[1][0] || box1[0][0] == box1[1][1]
        || box1[0][0] == box1[1][2] || box1[0][0] == box1[2][0] || box1[0][0] == box1[2][1] || box1[0][0] == box1[2][2]){
        return true;
    }
}

Avoid magic numbers

int box1[3][3];

for (int i = 0; i <= 2; i++){
    for (int j = 0; j <= 2; j++){

Rather than manually strewing 3s and 2s throughout the code, just declare a constant:

const int SQUARE_SIZE = 3;

Then you can say things like

int box1[SQUARE_SIZE][SQUARE_SIZE];

for (int i = 0; i < SQUARE_SIZE; i++) {
    for (int j = 0; j < SQUARE_SIZE; j++) {

Changing the <= to < allows use of the same constant in both places.

Why copy?

if (box1[0][0] != 0){
    if (box1[0][0] == box1[0][1] || box1[0][0] == box1[0][2] || box1[0][0] == box1[1][0] || box1[0][0] == box1[1][1]
        || box1[0][0] == box1[1][2] || box1[0][0] == box1[2][0] || box1[0][0] == box1[2][1] || box1[0][0] == box1[2][2]){
        return true;
    }
}

You copy a bunch of values from board to box1.  Why?

You could just replace all that with

if (board[0][0] != 0) {
    return false;
}

if (board[0][0] == board[0][1] || board[0][0] == board[0][2]
 || board[0][0] == board[1][0] || board[0][0] == board[1][1] || board[0][0] == board[1][2]
 || board[0][0] == board[2][0] || board[0][0] == board[2][1] || board[0][0] == board[2][2]) {
    return true;
}

Or the shorter

if (board[0][0] != 0) {
    return false;
}

for (int i = 0; i < SQUARE_SIZE; ++i) {
    for (int j = ((i > 0) ? 0 : 1); j < SQUARE_SIZE; ++j) {
        if (board[0][0] == board[i][j]) {
            return true;
        }
    }
}

return false;

Of course, that approach only really works on the corners.

Pick a different data structure or structures

You want to be able to answer the question:  is there more than one box with the same number?  So why not save the data in a structure than answers that?  E.g.

const int BOARD_SIZE = 9;

bool available_rows[BOARD_SIZE][BOARD_SIZE + 1];

for (int i = 0; i < BOARD_SIZE; ++i) {
    available_rows[i][0] = false;
    for (int j = 1; j <= BOARD_SIZE; ++j) {
        available_rows[i][j] = true;
    }
}

Then whenever you go to use a number, you say something like

if (available_rows[row][number] && available_columns[column][number] && available_squares[row/SQUARE_SIZE][column/SQUARE_SIZE][number]) {
    available_rows[row][number] = false;
    available_columns[column][number] = false;
    available_squares[row/SQUARE_SIZE][column/SQUARE_SIZE][number] = false;

    board[row][column] = number;
} else {
    // throw exception?  Or otherwise handle an invalid selection.
}

This does more work when selecting a number, but it self-maintains a valid board.  This is a common result in computer science.  It is often better to do work once and store it than to redo the same work repeatedly.

Note:  but not always.  :)

You can also build these structures inside an isValid function if you want.

bool isValid() {
    bool available_rows[BOARD_SIZE][BOARD_SIZE + 1];

    for (int i = 0; i < BOARD_SIZE; ++i) {
        available_rows[i][0] = false;
        for (int j = 1; j <= BOARD_SIZE; ++j) {
            available_rows[i][j] = true;
        }
    }

    bool available_columns[BOARD_SIZE][BOARD_SIZE + 1];

    for (int i = 0; i < BOARD_SIZE; ++i) {
        available_columns[i][0] = false;
        for (int j = 1; j <= BOARD_SIZE; ++j) {
            available_columns[i][j] = true;
        }
    }

    bool available_squares[SQUARES_PER_BOARD_SIDE][SQUARES_PER_BOARD_SIDE][BOARD_SIZE + 1];

    for (int i = 0; i < SQUARES_PER_BOARD_SIDE; ++i) {
        for (int j = 0; j < SQUARES_PER_BOARD_SIDE; ++j) {
            available_rows[i][j][0] = false;
            for (int k = 1; k <= BOARD_SIZE; ++k) {
                available_squares[i][j][k] = true;
            }
        }
    }

    for (int i = 0; i < BOARD_SIZE; ++i) {
        for (int j = 0; j < BOARD_SIZE; ++j) {
            if (board[i][j] == 0) {
                continue;
            }

            if (!available_rows[i][board[i][j]] || !available_columns[j][board[i][j]] || !available_squares[i/SQUARE_SIZE][j/SQUARE_SIZE][board[i][j]]) {
                return false;
            }

            available_rows[i][board[i][j]] = false;
            available_columns[j][board[i][j]] = false;
            available_squares[i][j][board[i][j]] = false;
        }
    }

    return true;
}

I'm assuming that this is inside a Board class, since you have direct access to the board variable in your original example.

This is off the top of my head.  As others have noted, there are many more examples of Sudoku implementations that have thought through the ramifications of each choice at greater length.  This is a common pattern that you should know, even if you choose not to use this particular solution here.


String Isolation Class As a lone developer I don't really have anyone to peer review my work in-house so I was hoping someone could take a look at this class and tell me in what ways I could improve it.

Essentially what this class does is receive a string from a socket that could possibly have multiple messages in it and isolates these messages based on the message terminator. In the isolate message method, it acquires the number of terminators with split and then loops through and adds the substring to the list of messages. Afterwards it removes that substring from the data string.

Off the top of my head the first opts I spot are using an array based off the terminator count instead of a list and making the terminator string array global so it's not making a new array each time.

internal class MessageHandler
{
    const string m_MessageTerminator = "<|>";
    int m_TerminatorLength = m_MessageTerminator.ToArray().Length;

    internal void ProcessStream(SocketState state)
    {
        ///Extract individual messages from socket message queue.
        ProcessMessages(IsolateMessages(state.MessageQueue.ToString()));

        //Find the last index of the message terminator and remove the substring from the string builder.
        //We do not want to clear the string builder because it could have partial messages inside.
        int lastTerminatorIndex = state.MessageQueue.ToString().LastIndexOf(m_MessageTerminator);
        state.MessageQueue.Remove
            (0, lastTerminatorIndex + m_TerminatorLength);
    }

    internal string[] IsolateMessages(string data)
    {
        List<string> messages = new List<string>();
        string[] terminators = new string[] { m_MessageTerminator };
        int terminatorCount = data.Split(terminators, StringSplitOptions.None).Length -1;

        for(int i = 0; i < terminatorCount; i++)
        {
            //Find The First Instance Of Message Termination
            int terminatorIndex = data.IndexOf(m_MessageTerminator);

            //Extract The Message Based On Terminator's Index
            //Then Remove The Terminator From The Message.
            messages.Add(data.Substring
                (0, terminatorIndex + m_TerminatorLength)
                    .Remove(terminatorIndex, m_TerminatorLength));

            //Remove The Message We Just Extracted From Overall Data Set
            data = data.Remove(0, terminatorIndex + m_TerminatorLength);
        }

        if (messages.Count > 0)
            return messages.ToArray();
        else
            return null;
    }

    internal void ProcessMessages(string[] messages)
    {
        if (messages == null)
            return;

        for(int i = 0; i < messages.Length; i++)
        {
            Console.WriteLine(messages[i]);
        }
    }
}

IsolateMessages()
You are creating a lot of strings here. Each time you call Substring(), or .Remove() on a string you are creating a new string. In addition the readability of

messages.Add(data.Substring
    (0, terminatorIndex + m_TerminatorLength)
        .Remove(terminatorIndex, m_TerminatorLength));

is very very bad.

By returning an IEnumerable<string> instead of string[] the execution of that method can be deffered by using yield.
By using the overloaded IndexOf(int, int) you could omit the terminatorCount and replace the for loop by a while loop.

Applying these two points would lead to this beautyful code
internal IEnumerable<string> IsolateMessages(string data)
{
    if (string.IsNullOrWhiteSpace(data)) { yield break; }

    var currentIndex = 0;
    var terminatorIndex = -1;

    while ((terminatorIndex = data.IndexOf(m_MessageTerminator, currentIndex)) > 0)
    {
        var messageLength = terminatorIndex - currentIndex;

        yield return data.Substring(currentIndex, messageLength);

        currentIndex = terminatorIndex + m_TerminatorLength;
    }
}

the only difference to your code is that if there is no message or the passed string data is empty my message isn't returning null so you could change the ProcessMessages() like
internal void ProcessMessages(IEnumerable<string> messages)
{
    foreach(var message in messages)
    {
        Console.WriteLine(message);
    }
}


Stock trading script The script uses a financial indicator called moving averages and their crossover points to recommend best actions for stock trading (buy vs sell).

starting_cash: initial cash position
prices: list of prices (ordered by time)
crossovers: list of crossover points on the prices, crossovers[i] is a list [time_index, buy_index]
buy_index = 1 means "buy" and buy_index = 2 means "sell"

I'm just looking to simplify the script but can provide more context if anyone wants it.

def make_trades(starting_cash, prices, crossovers):

    value = [starting_cash] #set value to starting_cash
    value = []  # creating empty set?
    stock_at = 0
    cash = starting_cash

    for i in range(0,len(prices)):
        added_value = (prices[i] - prices[i-1])/prices[i-1]
        try:
            time_index = crossovers[0][0]
            buy_index = crossovers[0][1]
            if i == time_index:
                if buy_index == 1:
                    stock_at = cash
                    cash = 0
                    value.append(stock_at)
                    crossovers = crossovers[1:]
                else:
                    if(stock_at == 0):
                        cash = value[i-1]
                        value.append(value[i-1])
                    else:
                        cash = value[i-1] + value[i-1]*added_value
                        stock_at = 0
                        value.append(cash)
                    crossovers = crossovers[1:]
            else:
                if cash == 0:
                    stock_at = value[i-1] + value[i-1]*added_value
                    value.append(stock_at)
                else:
                    value.append(cash)
        except (IndexError):
            added_value = (prices[i] - prices[i-1])/prices[i-1]
            if cash == 0:
                stock_at = value[i-1] + value[i-1]*added_value
                value.append(stock_at)
            else:
                value.append(cash)
    return [round(i, 2) for i in value]

In this code:

for i in range(0,len(prices)):
    added_value = (prices[i] - prices[i-1])/prices[i-1]

You start with i = 0. In that case, prices[i-1] will be prices[-1] which will access the last element in the prices list. That is, prices[len(prices) - 1].

I think this probably isn't what you want. You might want to do:

for i in range(1, len(prices)):

instead, so that prices[i-1] starts at zero. But then you won't run the loop on i=0, if that matters.

Alternatively, you might want to try:

last_price = 0
for i, price in enumerate(prices):
    added_value = 1.00 if last_price == 0 else (price - last_price)/last_price

    # ...
    last_price = price

Get rid of magic numbers

Your declaration that "buy_index == 1" means buy, and 2 means sell is ridiculous. If your code produces those numbers, then change your code. If you are reading the numbers from somewhere, change the numbers:

crossovers = ((t, 'buy' if a == 1 else 'sell') for t,a in crossovers)

You might use an enum instead, but if you're going to JSON, or reading from JSON, the string is probably easier.

Use modern Python idioms

One of the tenets of "modern Python" (3.x-style) is a bias towards iteration, especially the notion of infinite iterables. In other words:

Expect iterables, not sequences.
Generate, don't return a list.

I suggest that you convert prices to a series of (time, price) tuples, so that there is only one "source of truth" when it comes to what time should associate with a price.

Next, treat prices and crossovers as series, not lists: assume they are potentially infinite, and iterate them instead of trying to index them:

xo = iter(crossovers)
x_time, x_action = next(xo)

for time, price in prices:
    if time == x_time:
        if x_action == 'buy':
            #...
        x_time, x_action = next(xo)

Similarly, don't build a values = [] list and return it. Instead, yield your values when appropriate. And since they aren't indexed in a list, provide a timestamp to go along:

        yield time, round(cash, 2)
        # or
        yield time, round(stock_at, 2)


Splitting a list by predicate I have written a function splitAtPredicate that splits the list at element x satisfying p, dropping x and returning a tuple.
-- | 'splitAtPredicate', applied to a predicate @p@ and a list @xs@,
-- splits the list at element @x@ satisfying @p@, dropping @x@.
splitAtPredicate :: (a -> Bool) -> [a] -> ([a], [a])
splitAtPredicate p = splitAtPredicateAcc p []
  where
    splitAtPredicateAcc p left right@(x : xs')
      | null right = (left, right)
      | p x = (left, xs')
      | otherwise = splitAtPredicateAcc p (left ++ [x]) xs'

It works, but I'm new to Haskell, so I'm unsure how idiomatic and performant this is. In addition, I'm not too happy with the name splitAtPredicateAcc. Any suggestions are more than welcome.

Welcome to the Haskell community :)
Haskellers like composition. Your logic is composed of 2 parts:
splitWhen :: (a -> Bool) -> [a] -> [[a]]

toTuple :: [[a]] -> ([a], [a])

Let's address splitWhen first.
E.g. splitWhen (== '2') "132342245" should be ["13", "33", "", "45"].
To illustrate how it works:
initial state: (unconsumed input: "132332245", aggregator: [""])
step 1: (current input: '1', unconsumed input: "32342245", aggregator: ["1"])
step 2: (current input: '3', unconsumed input:  "2332245", aggregator: ["13"])
step 3: (current input: '2', unconsumed input:   "332245", aggregator: ["13",""])
step 4: (current input: '3', unconsumed input:    "32245", aggregator: ["13","3"])
step 5: (current input: '3', unconsumed input:     "2245", aggregator: ["13","33"])
step 5: (current input: '2', unconsumed input:      "245", aggregator: ["13","33", ""])
step 6: (current input: '2', unconsumed input:       "45", aggregator: ["13","33", "", ""])
step 7: (current input: '4', unconsumed input:        "5", aggregator: ["13","33", "", "4"])
step 8: (current input: '5', unconsumed input:         "", aggregator: ["13","33", "", "45"])

There are many ways to write this in Haskell, for example if we call the above logic f:
splitWhen :: (a -> Bool) -> [a] -> [[a]]
splitWhen p xs = f xs [] -- the initial aggregator
    where f [] agg = [agg]
          f (y : ys) agg = if p y
                           then agg : f ys [] -- we are ignoring the element here
                           else f ys (agg ++ [y]) -- put y into the aggregator

Notice the pattern match (y : ys), they are the current input and unconsumed input.

Notice the recursive function call of f.

toTuple is trivial:
toTuple :: [[a]] -> ([a], [a])
toTuple [] = ([], [])
toTuple [xs] = (xs, [])
toTuple (xs:ys:_) = (xs, ys)

Finally the exiting part - function composition:
splitAtPredicate :: (a -> Bool) -> [a] -> ([a], [a])
splitAtPredicate p = toTuple . splitWhen p

or if you are not yet comfortable with the pointfree style
splitAtPredicate :: (a -> Bool) -> [a] -> ([a], [a])
splitAtPredicate p xs = toTuple . splitWhen p xs

Because Haskell's lazy nature, splitAtPredicate will stop when it finds the second element that satisfies the predicate, as we have enough data to construct the pair.
To put everything together:
splitWhen :: (a -> Bool) -> [a] -> [[a]]
splitWhen p xs = f xs [] -- the initial aggregator
    where f [] agg = [agg]
          f (y : ys) agg = if p y
                           then agg : f ys [] -- we are ignoring the element here
                           else f ys (agg ++ [y]) -- put y into the aggregator

toTuple :: [[a]] -> ([a], [a])
toTuple [] = ([], [])
toTuple [xs] = (xs, [])
toTuple (xs:ys:_) = (xs, ys)

splitAtPredicate :: (a -> Bool) -> [a] -> ([a], [a])
splitAtPredicate p = toTuple . splitWhen p

Hope that helps :) And again welcome to the Haskell world.
If you haven't done it already, checkout https://hoogle.haskell.org , you'll love it :)


Picking 10 distinct words 'randomly' from List of unique words My goal is to pick 10 unique words randomly from a List containing 20 unique words.

I would remove the duplicate word from the List whenever a duplicate word is added and decrement the for loop counter by 1.

List<String> words = new ArrayList<String>();
/* code to add 20 words */

List<String> word10 = new ArrayList<String>();
for (i = 0; i < 10; i++) {
  int randomIndex = new Random().nextInt(words.size());
  if (word10.contains(words.get(randomIndex)) == true) {
    word10.remove(words.get(randomIndex));
    i--;
  }
  word10.add(words.get(randomIndex));
}
System.out.println("word10 List: " + word10);

Output

word10 List: [aah, abalone, aback, abandonee, abandonedly, abandon, abacus, aahed, aardvarks, abacuses]
word10 List: [a, aardvarks, aahing, abalones, abacuses, aahed, abacus, aah, abaft, abandoned]
word10 List: [abalones, ab, abaci, a, abacuses, abandonedly, aah, aahs, aardwolf, aardvark]

The code given is working properly. Is there a better approach?

First off, don't make a new Random instance every iteration.

Secondly your approach is ineffective for larger numbers, and especially if the number of words to choose is closer to the number of words available. The odds that a word is not already among the chosen words can become so small it can take a long time for your code to complete. Imagine picking 1.000.000 words in random order from a list of 1.000.000, getting the last word is a 1 in a million chance to get right.

A simple but more efficient approach is to shuffle the words and then just take the head of the list :

List<String> words = new ArrayList<String>();
/* code to add 20 words */

Collections.shuffle(words);

System.out.println("word10 List: " + words.subList(0, 10));

Obviously you don't need to shuffle the entire list. You can write your own partial Fisher-Yates shuffle (basically swap your choices to the head of the list), that stops once enough of the list is shuffled. This is definitely a good choice if you're going to do this for large collections of words with only a small list to choose.


Math formulas in Haskell I wrote some math formulas in Haskell and was wondering about how to clean the code up and make it more readable.

import Math.Gamma

pdf :: Double -> Double -> Double -> Double -> Double
pdf mu alpha beta x =
    ( beta / (2 * alpha * gamma  ( 1/beta) ) ) **
    exp ( -1* ( abs(x - mu )/alpha )) ** beta

cdf :: Double -> Double -> Double -> Double -> Double
cdf mu alpha beta x = 0.5 + signum(x - mu) * ( lowerGamma  (1/beta) ((abs(x-mu) / alpha)**beta) / (2 * gamma(1/beta)))

main = do
    let x = pdf 0 1 2 0.5
    print x
    let y = cdf 0 1 2 0.5
    print y

Your code looks fine to me. If you wish, you could use Greek letters. This makes the formula easier to read, but if you expect to modify it often, it may be too much trouble to enter the Greek letters. You might find it helpful to split up long formulas, as I've done for pdf. If you can think of more meaningful names than foo and bar, this might be a good idea. However, if this is a well-known formula in your field, splitting it up might actually make it less recognisable. It's a judgement call.

If you wrote this in literate Haskell, you could include a nicely-formatted version of the formula using LaTeX. That might be useful if you're writing code with a lot of formulas.

import Math.Gamma

γ :: Double -> Double
γ = gamma

pdf :: Double -> Double -> Double -> Double -> Double
pdf μ α β x = foo ** bar ** β
  where foo = ( β / (2 * α * γ  ( 1/β) ) )
        bar = exp ( -1* ( abs(x - μ )/α ))

cdf :: Double -> Double -> Double -> Double -> Double
cdf μ α β x = 0.5 + signum(x - μ) * ( lowerGamma  (1/β) ((abs(x-μ) / α)**β) / (2 * γ(1/β)))

main = do
    let x = pdf 0 1 2 0.5
    print x
    let y = cdf 0 1 2 0.5
    print y


Mocking the class under test with private method calls Consider the scenario below. It covers multiple methods of my unit under test.

public class OriginalSample
{
    public bool Foo(ISomeEntity entity)
    {
        return entity.IsThatSo ? entity.IsThatSo : Bar(entity);
    }

    public bool Bar(ISomeEntity entity)
    {
        return entity.IsThatAsWell;
    }
}

[TestClass]
public class OriginalSampleTest
{
    [TestMethod]
    public void Foo_EntityWithSo_ReturnsTrue
    {
        // Arrange
        Mock<ISomeEntity> someEntityMock = new Mock<ISomeEntity>();
        mock.SetupGet(m => m.IsThatSo).Returns( false );
        mock.SetupGet(m => m.IsThatAsWell).Returns( true );

        // Act
        private bool result = _OriginalSample.Foo( someEntityMock.Object );

        // Assert
        Assert.IsTrue(result);
    }
}

However, in my production code there is a lot that I need to Mock away that is then used in more method calls. I need to find a way to test the Foo method without hitting the Bar method.

I came up with the scenario as below. However the disadvantage is that i need a interface and pass the instance in each method. I do not like this design.

Any feedback?

public interface INewSample
{
    bool Foo(ISomeEntity entity, INewSample sample);
    bool Bar(ISomeEntity entity);
}

public class NewSample : INewSample
{
    public bool Foo(ISomeEntity entity, INewSample sample)
    {
        return entity.IsThatSo ? entity.IsThatSo : sample.Bar(entity);
    }

    public bool Bar(ISomeEntity entity)
    {
        return entity.IsThatAsWell;
    }
}

[TestClass]
public class NewSampleTest
{
    [TestMethod]
    public void Foo_EntityWithSo_ReturnsTrue
    {
        // Arrange
        Mock<ISomeEntity> someEntityMock = new Mock<ISomeEntity>();
        mock.SetupGet(m => m.IsThatSo).Returns( false );

        Mock<INewSample> sampleMock = new Mock<INewSample>();
        sampleMock.Setup(m => m.Bar).Returns(false).Verify();

        // Act
        private bool result = _OriginalSample.Foo( someEntityMock.Object, sampleMock.Object );

        // Assert (that the logic tried to use the 'bar' method
        sampleMock.Verify();
    }
}

You can't.

You need to ask yourself why it is you want to prevent executing Bar during testing.  If it is because you are worried about over-covering Bar during testing, stop worrying.  If it is because your real Bar implementation is long-running, then you probably need to inject another dependency into your class, or re-factor some code into a separate helper class which can then be injected.

As long as your Foo implementation needs to call Bar, your unit tests for Foo ultimately need to as well.  As long as Foo ultimately calls Bar, you cannot fully test Foo without executing Bar.  You shouldn't rely on it calling Bar, but you cannot prevent it from calling Bar in your tests without intentionally crippling your test coverage.


Update HTML4 to new HTML5 Semantic/Structural Elements I'm trying to update my website's code to HTML5's semantic and structural elements, but I'm not very sure with the sections and article element.

Here's the structure of my page:

                    <form action="user_login.php" method="post"></form>

                    <form action="user_register.php" method="post"></form>

Should I turn the main div and/or the content div into a section, and should the
 be the article, or should I put each of the <form> in an article?

Any other hints about the structure are also welcome.

You can take a look at the whole page here.

You can’t decide which sectioning elements to use without taking the actual content into consideration. So the following is meant for the home page of http://www.habbo-heaven.de/ (sub pages might differ!).

Your header (#outer-nav) is fine. It contains the h1 for the whole site. Therefore it is a child of body (and no other sectioning element).

Your nav (#outer-nav) is fine. It contains the site-wide navigation, therefore it is a child of body (and no other sectioning element).
You could use a ul for the links, though.

Using aside is fine. It contains content related to the whole page. Therefore it is a child of body (and no other sectioning element). However, your use of the headings inside of it is not correct for your content. The first heading inside the aside will be the (only!) heading of that sectioning element. But you use several h1 and, assuming that my understanding of your content is correct, there should be no top heading for all the blocks in the sidebars, because semantically they are all on the same level. There would be 2 possible solutions:

use one aside for all blocks and use section elements for each block (if you need containers for styling, use div elements around the aside elements)

Zuletzt gesetzte Gebote

Zuletzt hinzugefügte Möbel

Top Trader

Neusten User

Zuletzt hinzugefügte Angebote

use a separate aside for every block in the sidebar

Zuletzt gesetzte Gebote

Zuletzt hinzugefügte Möbel

Top Trader

Neusten User

Zuletzt hinzugefügte Angebote

Note that these two variants don’t generate the same document outline. The first variant will open another heading level for the aside, even if you don’t specify a heading explicitly. In the second variant all aside headings will be on the same (top) level as e.g. the main content. I’d prefer the first variant with one aside as container, because it doesn’t clutter the document outline and all your sidebar blocks are related content-wise anyway. If possible, find a suitable heading (you could hide it visually), like "Recent activity" or similar.

For the two forms you could use an article element for each (as you already do on the live site). section would work, too, of course. And it would also be possible to use no sectioning element at all (just headings), because you marked up all other content appropriately (header, footer, aside), so it’s clear what the main content is. For this content (just these two forms), it’s mostly a question of taste.

Your footer (#disclaimer) is fine. It contains the footer for the whole page/site, not only for the main content. Therefore it is a child of body (and no other sectioning element).


Interp1 linear extrap from Matlab to C++ I'm trying to implement what I think
griddedInterpolant(X,V,method) is doing inside Interp1.

I get the same results. Please review the extrapolation part as I'm not quite sure about it and comment about the algorithm correctness.

double Model::Interp1(const std::vector<double>& alpha, const std::vector<double>& varSurf, double detail_sensativity)
{
    //in case the interoplation point alraedy exists in the array
    //just return it
    std::vector<double>::const_iterator it = std::find(alpha.begin(), alpha.end(), detail_sensativity);
    if (it != alpha.end())
    {
        int index = it - alpha.begin();
        return varSurf[index];
    }
    else // we need linear interpolation/extrapolation.
    {
        double max = *std::max_element(alpha.begin(), alpha.end());
        double min = *std::min_element(alpha.begin(), alpha.end());
        if (detail_sensativity >= min && detail_sensativity <= max) //interpolate
        {
            for (size_t i = 0; i < alpha.size(); ++i)
            {
                if (detail_sensativity >= alpha[i] && detail_sensativity <= alpha[i + 1])
                {
                    //y = y0 + (y1-y0)*(x-x0)/(x1-x0);
                    double y0 = varSurf[i];
                    double y1 = varSurf[i + 1];
                    double x = detail_sensativity;
                    double x0 = alpha[i];
                    double x1 = alpha[i + 1];
                    double res = y0 + (y1 - y0)*(x - x0) / (x1 - x0);
                    return res;
                }
            }
        }
        else //extrapolate
        {
            //y = y0 + ((x - x0) / (x1 - x0)) * (y1 - y0)
            int i;
            if (detail_sensativity >= alpha.end)
            {
                i = varSurf.end() - varSurf.begin() - 1;
            }
            else
            {
                i = 0;
            }
            double y0 = varSurf[i];
            double y1 = varSurf[i + 1];
            double x = detail_sensativity;
            double x0 = alpha[i];
            double x1 = alpha[i + 1];
            double res = y0 + ((x - x0) / (x1 - x0)) * (y1 - y0);
            return res;
        }
    }
}

duplicated math

The two statements

double res = y0 + (y1 - y0)*(x - x0) / (x1 - x0);

and

double res = y0 + ((x - x0) / (x1 - x0)) * (y1 - y0);

are equivalent.

They both expand to

$$\frac{x y_0-x y_1+x_0 y_1-x_1 y_0}{x_0-x_1}$$

as can be seen in W\$\alpha \$ of the first and W\$\alpha \$ of the second (as the alternate forms are neatly in the same location, you can toggle between both tabs)

Even without the additional brackets around ((x - x0) / (x1 - x0)), it would still be the same thing.

That's somewhat plausible I'd say, because no matter if you are interpolating or extrapolating, you are still applying the formula for a line and that's the same whether you are between two points that define the line or outside of them.

duplicated code

For the code, that means the following block is a duplicate:

//y = y0 + (y1-y0)*(x-x0)/(x1-x0);
double y0 = varSurf[i];
double y1 = varSurf[i + 1];
double x = detail_sensativity;
double x0 = alpha[i];
double x1 = alpha[i + 1];
double res = y0 + (y1 - y0)*(x - x0) / (x1 - x0);
return res;

To simplify your code, move the block to the end. All code beforehand basically just figures out the right index i to use, then perform the code block. Here's an untested implementation of that. (only last else part of your code shown) // !!!!!!!!!! indicates lines of significant change.

else // we need linear interpolation/extrapolation.
{
    double max = *std::max_element(alpha.begin(), alpha.end());
    double min = *std::min_element(alpha.begin(), alpha.end());

    size_t i = 0;                                           // !!!!!!!!!!

    if (detail_sensativity >= min && detail_sensativity <= max) //interpolate
    {
        for (; i < alpha.size(); ++i)                       // !!!!!!!!!!
        {
            if (detail_sensativity >= alpha[i] && detail_sensativity <= alpha[i + 1])
            {
                break;                                      // !!!!!!!!!!
            }
        }
    }
    else //extrapolate
    {
        if (detail_sensativity >= alpha.end)
        {
            i = varSurf.end() - varSurf.begin() - 1;
        }
    }

    double y0 = varSurf[i];
    double y1 = varSurf[i + 1];
    double x = detail_sensativity;
    double x0 = alpha[i];
    double x1 = alpha[i + 1];
    double res = y0 + ((x - x0) / (x1 - x0)) * (y1 - y0);
    return res;
}

Changes I made:

moved intreaxtrapolation block to the end
used size_t i to top and used it for both cases
used break to end for loop. If you are a real C programmer, you'd shove all the condition of if (detail_sensativity >= alpha[i] && detail_sensativity <= alpha[i + 1]) into the condition of the for loop, just so you could have a loop without a body … just kidding, that wouldn't be very readable.

possible out of bounds error

Not sure if this is correct, just something looking suspicious:

Your loop ends at i < alpha.size();, thus i might end up being as big as alpha.size() - 1, which is the last valid index of the vector. However, you then do double x1 = alpha[i + 1];, which appears to be out of bounds in this case.

code organisation

You are writing a math utility function here. I would not put it into a Model namespace

         v--------------this
double Model::Inte....

I'm not a math expert, but I know that there are other ways to interpolate (cubic, bi-linear, Q-spline, slitherin, yoda, you name it). Maybe your Model asks for linear interpolation, but the function you are posting here is not a member function (assuming Model is a class). It doesn't depend on the state of an object and only uses the parameters passed to it (as far as I can tell). This looks like a free function and should possibly live in some Math namespace (or similar), independent from any Model that's using it.

Math is abstract and the same math is applied in many different fields in many different ways. Keep the abstraction by having the pure math functions separate and not tied to some Model that's interesting to your domain.

naming

It took me quite a while to figure out what the function is doing, because I was wondering what the heck detail_sensativity is supposed to be. Sure there's a typo in it, but still: what does it mean?

double Model::Interp1
(
    const std::vector<double>& alpha,
    const std::vector<double>& varSurf,
    double detail_sensativity
)

It's part of a bigger problem that's related to the former paragraph: you use domain specific names which are very cryptic to anybody outside your field.

I know what an interpolation function should be doing and with the clue that you are trying to duplicate griddedInterpolant(X,V,method) I knew what's going on. With your code alone, that would have been much harder. What is alpha? varSurf? detail_sensativity? And again, given there are many interpolation methods, what is Interp1?

Here's how this could look like:

double Interpolation::linear
(
    const std::vector<double>& x,
    const std::vector<double>& y,
    double x_value
)

simplification

I feel like this code could be simplified further. The case for extrapolation could be included in the for loop without being explicitly coded. After all, extrapolation just means using the same slope as the first and last interpolation steps. Checking max and min values feels like an overcomplication.

handling problematic input

At the moment, there are no error checks. One can happily pass in two std::vectors of different lengths and the algorithm will go bananas.
There should be some error checks.

Also, it could make a lot more sense to use different input parameter types. There's a pair class for example. The x and y values should always come in pairs, so why not do that? (code untested, just to illustrate)

double Interpolation::linear
(
    const std::vector<std::pair<double, double>>& x_y_points,
    double x_value
)

Now you only have to check if the length is > 1. I mean you're having a Model there, so you probably model something. I think std::pair<double, double> models the fact that the values come in pairs pretty well. Definitely more so than having two vectors. You can always provide an overload for the naysayers.


Angular directive test I'm learning how to write tests in Angular (these are my first tests in general). There are a lot of tutorials out there but each one of them has a different approach. I'm a bit confused if I'm doing things the right way.

I made a simple directive for testing purposes and I've called it tooltip (though its not really a tooltip).

tooltip.js:

angular.module('mytestapp', []);

(function(){
    angular.module('mytestapp').directive('tooltip', tooltip);

    function tooltip() {
        var directive = {
            controller: tooltipCtl,
            controllerAs: 'vm',
            restrict: 'E',
            template:
                "This is a tooltip for {{ name }}" +
                "<button ng-click='vm.myFun()'>Click me!</button>",
            scope: {
                name: '@'
            },
            link: link
        };

        return directive;

        function link(scope, elem, attrs) {}
    }

    function tooltipCtl() {
        var vm = this;

        vm.things = 3;

        vm.myFun = myFun;
        function myFun() {
          vm.age = 30;
        }
    }

})();

tooltip.spec.js:

describe('tooltip', function () {
    angular.mock.module('mytestapp');
    beforeEach(angular.mock.module('mytestapp'));

    describe('template', function () {
        var $compile;
        var $scope;
        var element;
        var controller;
        // beforeEach(module('templates'));

        beforeEach(inject(function(_$compile_, _$rootScope_) {
            $compile = _$compile_;
            scope = _$rootScope_.$new();
            element = angular.element("<tooltip name='john'></tooltip>");
            element = $compile(element)(scope);
            scope.$digest();
        }));

        beforeEach(function () {
            controller = element.controller('tooltip');
        });

        it('should have name equal to john', function (){
            expect(element.html()).toContain('john');
        });

        it('should have 3 items', function (){
            expect(controller.things).toBe(3);
        });

        it('should set age when clicked to 30', function () {
            expect(controller.myFun).toBeDefined();
            controller.myFun();
            expect(controller.age).toBe(30);
        });

    });
});

I'm using Karma and Jasmine, Angular 1.4.7, angular-mocks and ng-html2js. I'd be thankful for a general review of my code. Links to some good, up-to-date guides would help as well.

Additional questions:

I have ng-html2js included but I didn't really use it, can I use it in my example? Should I do it? Yes/no, and why?
I have a beforeEach(module('templates')); that I've used in some previous example but my spec works without it here. Is this module-templates things something common, or is there some sort of pattern where you use it?
I've used angular.mock.module but my spec works fine with just module. Any tips here?

You don't really need to use ng-html2js since you don't really have a file .html file.
Answered on question 1.
You can just use angular.mock.module if you want. I usually use this pattern for my tests.

Example:

const angular = window.angular,
  expect = window.chai.expect,
  sinon = window.sinon;

const inject = angular.mock.inject,
  module = angular.mock.module;

I have a repo with some patterns for unit testing. Feel feel to take a look at it.

https://github.com/rodoabad/angularjs-unit-testing-patterns


Using async / await with dynamic import() for ES6 modules I am experimenting with the best way to standardise my dynamic import() expressions when importing javascript modules.

import() returns a promise, so, to handle the asynchronous steps after theimport() expression, I can use either:

.then() syntax; or
async / await syntax

Whilst I am quite familiar with the older XHR2 approach and somewhat familiar with ES2015 .then() syntax, I am less familiar with async / await syntax.

I understand that I can only use await inside a function which has been declared or assigned with async.

But I am trying to understand why Approach 1 works.

Approach 1:

(async () => {

  let myImportedModule = await import('/path/to/my-module.js');

  myImportedModule.myFunction1();
  myImportedModule.myFunction2();

})();

But Approach 2 does not.

Approach 2:

(async () => {

  await import('/path/to/my-module.js');

  myFunction1();
  myFunction2();

})();

Additional Notes:

Clearly I have something wrong, but I thought Approach 2 would be the async / await equivalent of this snippet (using .then() syntax):

import('/path/to/my-module.js')

  .then((myImportedModule) => {

    myImportedModule.myFunction1();
    myImportedModule.myFunction2();

  });

but that it would enable me to access myFunction1 and myFunction2 directly, without needing to give a name to the resolved Promise.

Approach 2 does not work because accoring to MDN:

import '/modules/my-module.js';

Is how you import a module for Import a module for its side effects only

  Import an entire module for side effects only, without importing anything. This runs the module's global code, but doesn't actually import any values.

So, await import('/path/to/my-module.js'); won't actually import your myfunction1 and myFunction2 for you to use them, however, if you put an IIFE in there, it will be called.

If you don't want to use long names to call your function, you can destructure them :

(async () => {

  const { myFunction1, myFunction2 } = await import('/path/to/my-module.js');

  myFunction1();
  myFunction2();

})();

Or, call them in my-module.js and just import it and they will get called.

See the answers here for es6 import for side effects meaning.


JavaScript Objects- Can you confirm? Ok, I am in a JavaScript class, and I have no desire for anyone to do my schoolwork, I have already done what I think is correct, I am just hoping someone can take a look and tell me if I am close, or right, or whatever. Any feedback appreciated.  Also, the instructor was very specific in saying that if it asks for a line of code, then just do a line of code- so if it appears things are missing that would be why.

1.Write a line of code using the Location object to return the uniform resource locator (URL) of a Web page to a variable called myWebPage.

function getLocation(){
alert(document.location);}

2.Write a line of code using the Navigator object to return the Web browser name to a variable called myBrowserName.

function getLocation(){
alert(navigator.appName);}

3.Write a line of code using the Screen object to return the height of the display screen, not including operating system features such as the Windows Taskbar, to a variable called myScreenHeight

function getLocation(){
alert("Total height is : "+screen.height);}

4.Write a line of JavaScript code using the Window object and other properties to open a new Web browser window with www.google.com displaying and no menu bar

function getWindow(){
window.open('http://www.google.com','mywindow','width=400,height=200');}

That's it! Let me know if I did anything wrong, thank you!

It looks like you're not following the instructions strictly. For example, here:

  1.Write a line of code using the Location object to return the uniform resource locator (URL) of a Web page to a variable called myWebPage.

You didn't write a line of code, you wrote two, and you alerted the value instead of assigning to the variable. You also used a function, where the requirements do not include a function. I think the instructor is expecting something like this:

var myWebPage = window.location.href;


Displaying the date in MM/DD/YY format I spent about an hour and a half attempting to complete a problem from W3Resource. Here is what I came up with, which works. How can I shorten it?

function dateGet(){
var newDate = new Date();
var newMonth = newDate.getMonth() + 1;
var newDay = newDate.getDate();
var newYear = newDate.getFullYear();
var twoYear = (""+newYear).split("");
var combinedYear = twoYear[2] + twoYear[3];
var fullDate = 0;

if (newMonth < 10){
    fullDate += 0 + newMonth + '/';
} else {
    fullDate += newMonth + '/';
}

if (newDay < 10){
    fullDate += 0 + newDay + '/';
} else {
    fullDate += newDay + '/';
}

if (combinedYear < 10){
    fullDate += 0 + combinedYear;
} else {
    fullDate += combinedYear;
}

console.log(fullDate);

}

Whenever you find yourself writing code that feels the same, consider whether you can create a function to do it. For example consider using a function for padding to two digits.

Instead of turning newYear into a string so you can split it on digits, consider using the remainder operator (%). That is, var newYear = newDate.getFullYear() % 100;

Although (actually, because) javascript lets you play fast and loose with whether a variable is a string or a number or something else, it's worth avoiding switching between them where possible. It also has the slight advantage that it always gets the last two digits, even if the year had 3 or 5 digits in it. (Of course that won't be relevant for a very long time if it only prints the current date, but you shouldn't bank on that.)

I would be inclined to make this code slightly more general by allowing it to return rather than print out the formatted string, and allowing it to take a Date as a parameter rather than only using the current date. If you do want to print it, you can always do that with the output.

Of course that might actually be marginally longer, but I think that's worth it. Unless you're in a smallest number of characters competition, it is better to have readable, flexible code than short code if they conflict. Length is only a problem when there's too much there to keep in your head how it fits together.

Sticking strings together one bit at a time with + is an expensive thing to do. It's worth getting into the habit of using something like join As shown in jstudenski's code.


Analysis of multiple game scores I am trying to create a list based on some data, but the code I am using is very slow when I run it on large data. So I suspect I am not using all of the Python power for this task. Is there a more efficient and faster way of doing this in Python?
Here an explanantion of the code:

You can think of this problem as a list of games each with a list of participating teams and the scores for each team in the game. For each of the pairs in the current game it calculates the sum of the differences in score from the previous competitions (only for those competing!). Then it updates each pair in the current game with the difference in scores. Then it keeps track of the scores for each pair in each game and update this score as each game is played.

In the example below, based on some data, there are for-loops used to create a new variable list_zz.
The data and the for-loop code:
from collections import Counter, defaultdict
from itertools import combinations
import math

# test data
games = [['A', 'B'], ['B'], ['A', 'B', 'C', 'D', 'E'], ['B'], ['A', 'B', 'C'], ['A'], ['B', 'C'], ['A', 'B'], ['C', 'A', 'B'], ['A'], ['B', 'C']]

gamescores = [[1.0, 5.0], [3.0], [2.0, 7.0, 3.0, 1.0, 6.0], [3.0], [5.0, 2.0, 3.0], [1.0], [9.0, 3.0], [2.0, 7.0], [3.0, 6.0, 8.0], [2.0], [7.0, 9.0]]

list_zz= []

wd = defaultdict(Counter)
past_diffs = defaultdict(float)
this_diff = defaultdict(Counter)

for players, scores in zip(games, gamescores):
    if len(players) == 1:
        list_zz.append(math.nan)
        continue

    past_diffs.clear()
    this_diff.clear()

    for (player1, score1), (player2, score2) in combinations(zip(players, scores), 2):
        past_diffs[player1] += wd[player1][player2]
        past_diffs[player2] += wd[player2][player1]

        this_diff[player1][player2] = score1 - score2
        this_diff[player2][player1] = score2 - score1

    list_zz.extend(past_diffs[p] for p in players)

    for player in players:
        wd[player].update(this_diff[player])

print(list_zz)

Which looks like this:
[0.0,
 0.0,
 nan,
 -4.0,
 4.0,
 0.0,
 0.0,
 0.0,
 nan,
 -10.0,
 13.0,
 -3.0,
 nan,
 3.0,
 -3.0,
 -6.0,
 6.0,
 -10.0,
 -10.0,
 20.0,
 nan,
 14.0,
 -14.0]

Example to understand the code:
In the 5th game where A, B and C play, A gets -4 from the 1st game, 0 from the 2nd, -6 from the 3rd, and 0 from the 4th. Note that only A, B and C count in the 5th game. To be more clear A scores -4 in the 1st game, in the second he doesnt play so he scores 0, in the 3rd we count only the results for its competitors B and C which gives -6, and in the 4th he doesnt play so he gets 0. Notices that results are from the past games against current competitors.
If you could elaborate on the code to make it more efficient and execute faster, I would really appreciate it.

Solve with math
This is a math problem. Let say we have a competition: [a, b, c] score [5, 2, 10], this means that the scoring is:
$$
\begin{array}{|l|r|r|r|}
& \textrm{a} & \textrm{b} & \textrm{c} & \textrm{res} \\
\hline
\textrm{a} & \text{NaN} & 3 & -5 & -2 \\
\textrm{b} & -3 & \text{NaN} & -8 & -11\\
\textrm{c} & 5 & 8 & \text{NaN} & 13
\end{array}
$$
As you should be able to see, you don't need to calculate the sum again and again for each pair.
Solution:
For each team:
\$\text{team's score} \times \text{number of teams} - \text{total score}\$.
score[a] =  5 * 3 - 17 =  -2
score[b] =  2 * 3 - 17 = -11
score[c] = 10 * 3 - 17 =  13

The time complexity of this is \$O(n)\$. Calculate all pairs is \$O(n^2)\$.
Some code
Here I will save the total score for each team (not the history of the scores _  the code change for that would not be big tho).
from collections import Counter, defaultdict

# test data
games = [['A', 'B'], ['B'], ['A', 'B', 'C', 'D', 'E'], ['B'], ['A', 'B', 'C'], ['A'], ['B', 'C'], ['A', 'B'],
         ['C', 'A', 'B'], ['A'], ['B', 'C']]

gamescores = [[1.0, 5.0], [3.0], [2.0, 7.0, 3.0, 1.0, 6.0], [3.0], [5.0, 2.0, 3.0], [1.0], [9.0, 3.0], [2.0, 7.0],
              [3.0, 6.0, 8.0], [2.0], [7.0, 9.0]]

wd = defaultdict(float)

for players, scores in zip(games, gamescores):
    if len(players) == 1:
        continue
    total_sum = sum(scores)
    for player, score in zip(players, scores):
        wd[player] = wd[player] + score * len(scores) - total_sum

print(wd)

Result
defaultdict(<class 'float'>, {'A': -12.0, 'B': 32.0, 'C': -17.0, 'D': -14.0, 'E': 11.0})

Edit: Grouping based on last results
OP clarified that each competition affect the total score taking from the previous competition because the grouping changes.
In the example, scores: [1.0, 5.0], [3.0], [2.0, 7.0, 3.0, 1.0, 6.0], [3.0], [5.0, 2.0, 3.0], [1.0], [9.0, 3.0], teams: [a,b], [b], [a,b,c,d,e], [b], [a,b,c],
A scores as:

1st game: -4
2nd game:  0 (he was not participant)
3rd game: -6 (because at 5th game, only A, B, C are competing)

For that, we can pre-process the groups to make sure that only the competitors of next game are considered.
Pre-processing idea
That is just an example how to solve the issue using the pre-processing. Notice that is a backward thinking. Next game determines which competitors matter in terms of scoring. Therefore, the processing is done in reverse order.
def pre_process(games, gamescores):
    last_game = {}
    result = []
    for game in zip(reversed(games), reversed(gamescores)):
        game_dict = dict(zip(game[0], game[1]))
        if len(game[0]) == 1:
            result.append(game_dict)
            continue
        if len(last_game) != 0:
            union_set = set(game_dict.keys()).intersection(set(last_game.keys()))
            last_game = game_dict
            game_dict = {k: game_dict[k] for k in union_set}
        else:
            last_game = game_dict
        result.append(game_dict)
    return result

pairs = list(reversed(pre_process(games, gamescores)))
wd = defaultdict(float)
for game in pairs:
    players = list(game.keys())
    scores = [game[k] for k in players]
    if len(players) == 1:
        continue

    total_sum = sum(scores)
    for player, score in zip(players, scores):
        wd[player] += score * len(scores) - total_sum
    print(wd)


Idiomatic approach to applying a keyword replace algorithm to a Java 8 Stream I have an approach that seems to work though I feel like I must be misusing something to achieve it.  I am implementing a keyword matching algorithm to a Java 8 IntStream.  The goal is to walk over a stream of char values (created by calling .chars() on an input string called sentence).  If a keyword matches it is returned with its replacement value otherwise the original characters are returned unmodified.  The code I have written so far looks like this:  The Replacer is implemented as a Consumer<Character> with some stateful properties:

class Replacer implements Consumer<Character> {
    StringBuffer out;
    StringBuffer buffer;
    KeywordTrieNode current_keyword_trie_node;
    KeywordTrieNode keyword_trie_root;

    public Replacer(KeywordTrieNode keyword_trie_root) {
        this.keyword_trie_root = keyword_trie_root;
        this.current_keyword_trie_node = keyword_trie_root;
        this.out = new StringBuffer();
        this.buffer = new StringBuffer();
    }

    @Override
    public void accept(Character c) {
        KeywordTrieNode node = current_keyword_trie_node.get(c);
        if (node != null) {
            buffer.append(c);
            current_keyword_trie_node = node;
        } else {
            String keyword = current_keyword_trie_node.get();
            if (keyword != null) {
                out.append(keyword);
            } else {
                out.append(buffer);
            }
            out.append(c);
            buffer = new StringBuffer();
            current_keyword_trie_node = this.keyword_trie_root;
        }
    }

    @Override
    public String toString() {
        // Flush the buffer
        String keyword = current_keyword_trie_node.get();
        if (keyword == null) {
            out.append(buffer);
        } else {
            out.append(keyword);
        }
        return out.toString();
    }
}

Then to use this I have this function:

public String replace(String sentance) {
    return replace(sentance.chars());
}
public String replace(IntStream stream) {
    Replacer replacer = new Replacer(this.keyword_trie_root);

    stream.mapToObj(c -> (char) c).forEachOrdered(replacer);

    return replacer.toString();
}

Here the String is converted into a IntStream, the stream is then mapped into a stream of char values (which are autoboxed into Character which is needed for the underlying Map inside the keyword_trie_root).  Then the replacer is passed to forEachOrdered allowing it to see each character and build a bigger and bigger StringBuffer until it's toString() is invoked (performing one final buffer flush).  This doesn't feel like the correct use of Java streams but I'm hitting a bit of a wall when trying to reimagine this as a Combiner which I think is the correct way to solve this problem.  Any ideas?

Bug:

Calling toString  repeatedly will append the last keyword (or the buffer) multiple times. This is not really in line with how this should work.

Conventions:

Java coding conventions state that member names should be lowerCamelCase. the fields related to the keyword trie do not follow that convention.

Another convention that you seem to disregard here is that Tries usually enable quick searching through strings by virtue of allowing skipping characters before doing the next comparison. As such naming the KeywordTrie as "Trie" is not really correct.

Minor simplifications & fixes:

You should specify explicit visibilities on all your fields. It's almost never appropriate to use the default visibility in java. package-private is almost always wrong.

The names you're using are somewhat wordy for my own taste as well.

In addition you can initialize fields in a field initializer instead of the constructor to make the ctor simpler to read. Consider:

public class Replacer [...] {
    private StringBuffer out = new StringBuffer();
    private StringBuffer buffer = new StringBuffer();
    private KeywordTrieNode currentNode;
    private KeywordTrieNode trieRoot;

Design issues:

A consumer should not return a result. That's not what they do. They take an element and that's it.

What you have here is a Collector. Something that takes a Stream and collapses it into a value. Let's make sure we use the correct interface here:

public class Replacer implements Collector<Character, StringBuilder, String> {

This in turn gives us the following functions to implement:

BiConsumer<StringBuffer, Character> accumulator()
Set<Consumer.Characteristics> characteristics()
BinaryOperator<StringBuffer> combiner()
Function<StringBuffer, String> finisher()
Supplier<StringBuffer> supplier()

Let's start with the important stuff that governs how the Collector will be used by the Stream implementation. Our collector is Ordered, Sequential and changes stuff when the finisher is invoked. This means that none of the Characteristics for optimization apply:

public Set<Collector.Characteristics> characteristics() {
    return Collections.emptySet();
}

Now we can deal with the stuff that we need to set up:

public BinaryOperator<StringBuffer> combiner() {
    // explicitly doesn't do anything, supplier always returns the same object
    return (a, b) -> a;
}

public Supplier<StringBuffer> supplier() {
    return () -> {
        return out;
    };
}

Note that I assume we're using the same private fields (and constructor) as in the question, because they seem like an acceptable encapsulation of the problem you're trying to solve.

Now we need to implement the accumulation, this is basically your "accept":

public BiConsumer<StringBuffer, Character> accumulator() {
    return (out, character) -> {
        KeywordTrieNode next = currentNode.get(character);
        if (next != null) {
            buffer.append(character);
            currentNode = next;
        } else {
            String keyword = currentNode.get();
            out.append(keyword == null ? buffer : keyword)
               .append(character);
            buffer = new StringBuilder();
            currentNode = trieRoot;
        }
    };
};

public Function<StringBuffer, String> finisher() {
    return (out) -> {
        String keyword = currentNode.get();
        out.append(keyword == null ? buffer : keyword);
        return out.toString();
    };
}

With this implementation we can rewrite your replace function to look as follows:

public String replace(IntStream chars) {
     return chars.collect(new Replacer(keywordTrieRoot));
}


Multithreaded Matrix Multiplication in C++, improving efficiency? Here is a very simple Matrix Multiplication console application using multiple threads (one per row), I'm just looking for feedback to keep improving :) thank you in advance
    #include <iostream>
    #include <fstream>
    #include <sstream>
    #include <vector>
    #include <string>
    #include <thread>

using namespace std;

struct Matrix {
    unsigned int cols = 0;
    unsigned rows = 0;
    int **data;

    Matrix() {};
    Matrix(int r, int c) {
        cols = c;
        rows = r;
        data = new int* [rows];
        for (int i = 0; i < rows; ++i) {
            data[i] = new int[cols];
            for (int j = 0; j < cols; ++j)
                data[i][j] = 0;
        }
    }
};

Matrix ReadMatrix(string fileName) {
    fstream file(fileName, fstream::in);
    Matrix matrix;
    try {
        if (file.is_open()) {
            int rows, cols;
            file >> rows >> cols;
            matrix = Matrix(rows, cols);

            for (int r = 0; r < matrix.rows; ++r)
                for (int c = 0; c < matrix.cols; ++c)
                    file >> matrix.data[r][c];

            file.close();
            cout << "File: " << fileName << " read.\n";
        }
        else cout << "could not open file/matrix: " << fileName << "\n";
        return matrix;
    }
    catch (exception ex) {
        if (file.is_open())file.close();
        throw ex;
    }
}

void MultiplyRow(Matrix m1, Matrix m2, Matrix result, int row) {
    for (int c = 0; c < m2.cols; ++c)
        for (int i = 0; i < m1.cols; ++i)
            result.data[row][c] += (m1.data[row][i] * m2.data[i][c]);
    string str = ("Row: " + to_string(row) + " complete.\n");
    cout << str;
}

Matrix MultiplyMatrices(Matrix m1, Matrix m2) {
    try {
        Matrix matrix;
        if (m1.cols == m2.rows) {

            vector<thread> threads;
            matrix = Matrix(m1.rows, m2.cols);

            for (int i = 0; i < m1.rows; ++i)
                threads.emplace_back(MultiplyRow, m1, m2, matrix, i);

            for (int i = 0; i < threads.size(); ++i)
                threads[i].join();

            cout << "Matrices multiplied.\n";
        }
        else cout << "cannot multiply matrices\n";
        return matrix;
    }
    catch (exception ex) {
        throw ex;
    }
}

void WriteMatrixToFile(int **data, int rows, int cols, string fileName) {
    try {
        ofstream file(fileName);
        char SPACE = ' ';
        if (file.is_open()) {
            file << rows << SPACE << cols << "\n";
            for (int r = 0; r < rows; ++r) {
                for (int c = 0; c < cols; ++c) {
                    file << data[r][c] << SPACE;
                }
                file << "\n";
            }
        }
        cout << "Matrix written to: " << fileName << ".\n";
        file.close();
    }
    catch (exception ex) {
        throw ex;
    }
}

int main()
{
    //read matrices
    Matrix matrix1 = ReadMatrix("matrix1.txt");
    Matrix matrix2 = ReadMatrix("matrix2.txt");

    //multiply matrices
    Matrix result = MultiplyMatrices(matrix1, matrix2);

    //put result into results file
    WriteMatrixToFile(result.data, result.rows, result.cols, "result.txt");
}

Welcome to Code Review!
Algorithm
Currently, a thread is started for every row of the result matrix.  This works best if the matrix has a large number of columns and the number of rows is close to the number of native threads.
A more efficient strategy in general might be to partition the rows to several chunks according to the native thread count instead.  Without careful benchmarking, it's hard to say for sure.
Encapsulation
Matrix has a class invariant — for a non-empty matrix, data points to an array of rows pointers that point to arrays of cols elements each.  To ensure synchronization, the data members can be kept private.
Memory issues
Matrix fails to observe the rule of zero:

Classes that have custom destructors, copy/move constructors or
copy/move assignment operators should deal exclusively with ownership
(which follows from the Single Responsibility Principle). Other
classes should not have custom destructors, copy/move constructors or
copy/move assignment operators.
This rule also appears in the C++ Core Guidelines as C.20: If you can
avoid defining default operations, do.

To fix this issue, simply change the pointers to std::vector.
Storage of matrices
Currently, you are storing a pointer to a dynamic array of pointers, which, in turn, point to dynamic arrays, incurring double indirection overhead and causing memory segmentation.  A more efficient method is to store all elements in a flattened array and provide a helper function to access the element at a given place:
class Matrix {
    std::vector<int> elements;
    std::size_t n_rows;
    std::size_t n_cols;
public:
    Matrix()
        : Matrix(0, 0)
    {
    }
    Matrix(std::size_t n_rows, std::size_t n_cols)
        : elements(n_rows * n_cols), n_rows(n_rows), n_cols(n_cols)
    {
    }

    std::size_t row_count() const noexcept {
        return n_rows;
    }
    std::size_t col_count() const noexcept {
        return n_cols;
    }

    int& operator()(std::size_t i, std::size_t j) {
        return elements[i * n_cols + j];
    }
    const int& operator()(std::size_t i, std::size_t j) const {
        return elements[i * n_cols + j];
    }
};

Note that this does not preserve the aliasing semantics of your Matrix and performs deep copying instead, so the by-value parameters should be changed to references.
Miscellaneous
using namespace std; is considered bad practice in C++.
If you turn on high warning levels, you will see warnings about mixing signed and unsigned integers in comparisons, which is because you are mixing int and unsigned int.  Instead of them, use std::size_t to store sizes, which is what the language functionalities and the standard library use.
In ReadMatrix, file streams close automatically when they go out of scope, so there's no need to do that manually.  std::ifstream can be used to simplify the construction of the file stream.
Instead of printing messages, throw exceptions when errors occur.


Merging multiple JSON files using Python I have multiple (1000+) JSON files each of which contain a JSON array. I want to merge all these files into a single file.

I came up with the following, which reads each of those files and creates a new object with all the contents. I then write this new object into a new file.

Is this approach efficient? Is there a better way to do so?

head = []
with open("result.json", "w") as outfile:
    for f in file_list:
        with open(f, 'rb') as infile:
            file_data = json.load(infile)
            head += file_data
    json.dump(head, outfile)

First off, if you want reusability, turn this into a function. The function should have it's respective arguments.
Secondly, instead of allocating a variable to store all of the JSON data to write, I'd recommend directly writing the contents of each of the files directly to the merged file. This will help prevent issues with memory.
Finally, I just have a few nitpicky tips on your variable naming. Preferably, head should have a name more along the lines of merged_files, and you shouldn't be using f as an iterator variable. Something like json_file would be better.


Keyword classification in Scala... simple? I am pretty newbie in Scala, my background expertise is basically Java and Python, and I'd like to know your thoughts about this code snippet written in Scala to know if this is a "regular piece of code written in Scala" or if this is something kind of "obfuscated" and overcomplicated.
The code just implements a simple decision flow to categorize keywords depending on a few rules.
Thanks!
def preprocess(kwd: Keyword): Option[PrepKeyword] = {

val isFoo = kwd.name.exists(_.toLowerCase(locale) == "foo")
    for {
      filterCategory <- (isFoo, kwd.type) match {
                case (true, _)  => Some(false)
                case (false, 1) => Some(true)
                case (false, 2) => Some(false)
                case _          => None
              }
      fooName <- kwd.name match {
                        case None | Some("name1")                           => None
                        case Some("myCategory") if filterCategory           => Some("GreatCategory")
                        case other                                          => other
                      }
    } yield PrepKeyword(
      keyword = kwd,
      name = fooName,
      path = if (isFoo) "Foo1" else "Foo2"
    )
  }

The posted code won't compile without a working definition of Keyword and PrepKeyword. And type is a reserved word so kwd.type isn't allowed (might be version dependent).
The code, as presented, is inconsistent or just plain wrong in its indenting but, other than that, it's reasonably clear in intent and presentation.
The logic is, perhaps, overly convoluted. I found that I had to go back and forth to see how output was built from input. I eventually decided that kwd.name was the key value from which most everything else flows. With that in mind I tried to rebuild the logic.
def preprocess(kwd: Keyword): Option[PrepKeyword] = {
  val fooName = "(?i)(foo)".r
  kwd.name.flatMap{
    case "name1"      => None
    case "myCategory" =>
      kwd.typ match {
        case 1 => Some(PrepKeyword(kwd, "GreatCategory", "Foo2"))
        case 2 => Some(PrepKeyword(kwd, "myCategory", "Foo2"))
        case _ => None
      }
    case fooName(fnm) => Some(PrepKeyword(kwd, fnm, "Foo1"))
    case nonFoo       => Some(PrepKeyword(kwd, nonFoo, "Foo2"))
  }
}

This, according to my tests so far, produces the same results. It builds the output in multiple places but in each case it's pretty easy to see how the input directed the output.


Is my JavaScript test actually testing anything worthwhile here Given a production module like this:

function ($) {
   var
    showForm = function () {
        $.get(url, function (html) {
            $('body').append(html);
        });
    };

   return {
       showForm: showForm
   };
});

I have written the following unit test:

it("calling showForm appends html to existing page", function () {

    var testPage = $('

');
    var expectedPage = $('

');
    var htmlToAppend = $('');

    var callback = function () { return testPage.append(htmlToAppend); }

    sinon.stub(presenter, "showForm").yields(testPage);

    presenter.showForm(callback);

    expect(testPage.html()).to.equal(expectedPage.html());
});

I am trying to test that when presenter.showForm is called, the returned html is appended successfully appended.  Problem is, I'm not usre if it's a valid test as pretty much everything is fake.

I'm toying with the idea of abstracting out the call to $('body').append(html); to another public function of the module or perhaps even to another module so that
the call presenter.showForm(callback); can use the real function but I'm not sure?

I hope that all makes sense.

Can someone review and advise please?

I'm not quite sure what you mean by "abstracting out the call...", but given that the "production module" actually looks like that, then I'd say that you have a perfectly valid little test there.

It is good to hear that you really are interested in making sure that your test cases are testing the right things. Unit tests are too often something that programmers write simply because they are forced to do it, and they are therefore happy enough when the tests don't fail - not pausing to consider that it might have passed because the test was incorrect, tested the wrong criteria, or did not in fact test anything at all.

Using fake data is what you normally do at this level of functionality, so you are good there. You might generally want to consider using more than one set of test data, but in this case I would say that it is not necessary.


Simple GCD utility in Java I previously discussed the performance concerns about different GCD algorithms. I wrote a simple Java class that implements the Binary GCD algorithm. It is a really tiny class that has only two easy-to-use methods. See for yourself:

GCD.java

package math;

/**
 * This is a utility class that provides the functionality of calculating the
 * GCD or Greatest Common Divisor (also known as HCF or Highest Common
 * Factor) of two or more integers.
 *

 * It has only two methods:
 *

 *
{@link #of(long, long)}

 *
{@link #of(long, long, long...)}

 *

 * Usage is very simple:
 *

 * ...
 * // assuming a, b, c, d and e are integers (long, int, etc.)
 * long gcd1 = GCD.of(a, b);
 * long gcd2 = GCD.of(a, b, c);
 * long gcd3 = GCD.of(a, b, c, d);
 * long gcd4 = GCD.of(a, b, c, e);
 * ...
 *

 *
 * @author Subhomoy Haldar
 * @version 1.0
 */
public class GCD {

    /**
     * Private constructor to prevent instantiation.
     */
    private GCD() {
        throw new Error("Instantiation not allowed.");
    }

    /**
     * This method computes the GCD of two positive integers using the Binary
     * GCD algorithm.
     *

     * This method does not complain if the integers provided are negative.
     * In such a case, it simply the absolute value and performs the
     * computation. If one of the arguments is zero, the other argument is
     * returned. If both are zero, then zero is returned.
     *
     * @param a The first positive integer.
     * @param b The second positive integer.
     * @return The GCD of two positive integers.
     */
    public static long of(long a, long b) {
        // corner cases
        if (a == 0) return b;
        if (b == 0) return a;
        if (a < 0) a = -a;
        if (b < 0) b = -b;
        // number of tailing zeroes = power of 2 present
        int a0 = Long.numberOfTrailingZeros(a);
        int b0 = Long.numberOfTrailingZeros(b);
        // extract the the number of trailing zeroes common to them
        int commonPower = a0 < b0 ? a0 : b0;
        // make them odd
        a >>>= a0;
        b >>>= b0;
        while (a != b) {
            if (a > b) {
                a -= b;
                a >>>= Long.numberOfTrailingZeros(a);
            } else {
                b -= a;
                b >>>= Long.numberOfTrailingZeros(b);
            }
        }
        return a << commonPower; // multiply back the common power of 2
    }

    /**
     * This method returns the GCD of all the integers provided.
     *

     * This method makes use of the {@link #of(long, long) of(long, long)}
     * method.
     *
     * @param a      The first positive integer.
     * @param b      The second positive integer.
     * @param others The other positive integers.
     * @return The GCD of all the integers provided.
     * @see #of(long, long) Internally used by this method.
     */
    public static long of(long a, long b, long... others) {
        long gcd = of(a, b);
        for (long number : others) {
            gcd = of(gcd, number);
        }
        return gcd;
    }
}

I welcome comments on any aspect of the code. This class will be reused in future questions.

Instead of throwing an Error, an AssertionError would be better.
An Error is too broad.
An AssertionError conveys better that instantiating is an error by the programmer, violating a precondition.
(The only way this error will be triggered is if you try to create an instance within this class.)

private GCD() {
    throw new AssertionError("Instantiation not allowed.");
}

As this class has only private constructors,
it cannot be extended:
derived classes would need to call a constructor in the super-class,
but none are available.
To make the intention perfectly clear that this class is not designed to be extended, it's good to make it final.


Reading words until an empty line This following code executes correctly.I was wondering if there are better ways to write this code instead of using while(true) and break.For example using expressions,flags, do-while,etc.

import java.util.ArrayList;
import java.util.Scanner;

public class Words {
public static void main(String[] args) {
    Scanner reader = new Scanner(System.in);
    ArrayList<String> words = new ArrayList<String>();

while(true){
    System.out.print("Type a word: ");
    String word = reader.nextLine();
    words.add(word);

if(word.isEmpty()){
    System.out.println("You typed the following words:");
    for(String result: words){
    System.out.println(result);
    }
    break;
    }
 }
}
}

There are a few things in here that make sense. The first one is to use a function, and to return a List of words from that. The next thing is to use a do-while loop instead of a while loop... actually, truth is that I would personally use an assignment in the while condition. This is relatively uncommon, so let me explain:

private static List<String> readWords(Scanner source) {
    String word = null;
    List<String> words = new ArrayList<>();
    while (!(word = source.nextLine()).isEmpty()) {
        words.add(word);
    }
    return words;
}

That's a relatively complicated loop. It puts both the readline, and the loop-check in the same place. It reads the line in to word, then also makes sure that is not empty.

While the above is relatively uncommon to see, it makes the conditional really concise, and reduces duplicate checks.

Oh, and using a forEach on the resulting list makes the print really easy....

public static void main(String[] args) {
    List<String> words = readWords(new Scanner(System.in));
    words.forEach(System.out::println);
}


Code to get the IP address and format it to run in nmap For scanning my subnet using my IP address, I need to run the following command

nmap -sn 192.168.100.*

I want to extend this functionality such that I don't have to manually specify the first three parts of the IP range myself. I came up with this rough solution.

ip=$( hostname -I | awk '{print $1}' | awk -F. '{print $1; print $2; print $3}' ORS="."; echo -n "*");
nmap -sn $ip

Is there a more cleaner way of achieving the same?

You can do it purely in Bash, using ${parameter%word} to strip off the last octet from the IP address.

hostname -I | while read ip _ ; do
    nmap -sn  ${ip%.*}.\*
done

Note that hostname -I is not portable: the -I option appears to be a GNU extension.


Class for user actions I implemented a class for relationships with the user (deletion, name change, check for existence), I created this class to remove extra logic from the activity class. Could you talk about how my code can be improved?

class UserActionManager @Inject constructor(private val repository : BaseAppRepository,
                                            private val sharedPreferences : JsonSharedPreferences) {

    fun editUserName(user: User,newUserName: String) {
        Completable.fromAction {
            val userObjective:Objective? = sharedPreferences.loadObject(GlobalConstants.OBJECTIVE_SAVE_KEY + user.userName,Objective::class.java)
            val userProfile:Profile? = sharedPreferences.loadObject(GlobalConstants.PROFILE_SAVE_KEY + user.userName,Profile::class.java)
            sharedPreferences.saveObject<Objective>(GlobalConstants.OBJECTIVE_SAVE_KEY + newUserName, userObjective)
            sharedPreferences.saveObject<Profile>(GlobalConstants.PROFILE_SAVE_KEY + newUserName, userProfile)
            repository.updateUserName(user.userName, newUserName)
            user.userName = newUserName
            repository.updateUser(user)
        }.subscribeOn(Schedulers.io()).subscribe()
    }

    fun deleteUser(user: User) {
        Completable.fromAction { repository.deleteUser(user) }.subscribeOn(Schedulers.io()).subscribe()
        sharedPreferences.saveObject<Objective>(GlobalConstants.OBJECTIVE_SAVE_KEY + user.userName, null)
        sharedPreferences.saveObject<Profile>(GlobalConstants.PROFILE_SAVE_KEY + user.userName, null)
    }

    fun isEmptyUser(userName: String): Single<Boolean> {
        return repository.getUserByUserName(userName)
                .subscribeOn(Schedulers.io())
                .isEmpty
    }
}

Extract your preferences code into extra class or create extension functions to get those objects. Ex. sharedPreferences.loadObject(GlobalConstants.OBJECTIVE_SAVE_KEY + user.userName,Objective::class.java) should have it's own method. Ex. loadUserByName.
You are using kotlin, use it's strengths like non-nullables by default. Does repository.updateUserName() really accept nullables? That seems wrong. I'd use it for example with elvis operator loadUserByName(user.username) ?: error("username not found")


Strategy design pattern with various duck type classes I've recently picked up the Head First Design Patterns book in an effort to become a more efficient and better Python programmer. Unfortunately, the code examples in this book are in Java.

I'm not the first one that wishes there was a Python version of this book, but I thought that writing the Python code would be a great exercise.

I've tried to implement the Strategy design pattern from Chapter 1 of Head First Design Patterns below in Python. Now, I know that Python is not Java in a general sense, but not in the nitty-gritty sense. So, I'm sure there are more Pythonic things that I can do to this code.

Off the top of my head we could probably implement the Duck class set_fly_behavior() and set_quack_behavior() using the Python property built-in.

Any other suggestions?

#!/usr/bin/env python

################################################################################
# Abstract Duck class and concrete Duck type classes.
################################################################################
class Duck(object):
    def __init__(self):
        pass

    def set_fly_behavior(self, fb):
        self.fly_behavior = fb

    def set_quack_behavior(self, qb):
        self.quack_behavior = qb

    def perform_fly(self):
        self.fly_behavior.fly()

    def perform_quack(self):
        self.quack_behavior.quack()

    def swim(self):
        print "All ducks float, even decoys!"

class MallardDuck(Duck):
    def __init__(self):
        Duck.__init__(self)
        fly_instance = Fly()
        self.set_fly_behavior(fly_instance)
        quack_instance = Quack()
        self.set_quack_behavior(quack_instance)
        print "I'm a real Mallard Duck"

class ModelDuck(Duck):
    def __init__(self):
        Duck.__init__(self)
        fly_no_way_instance = FlyNoWay()
        self.set_fly_behavior(fly_no_way_instance)
        squeak_instance = SqueakQuack()
        self.set_quack_behavior(squeak_instance)
        print "I'm a Model Duck"

class DecoyDuck(Duck):
    def __init__(self):
        Duck.__init__(self)
        fly_no_way_instance = FlyNoWay()
        self.set_fly_behavior(fly_no_way_instance)
        mute_instance = MuteQuack()
        self.set_quack_behavior(mute_instance)
        print "I'm a Decoy Duck"

################################################################################
# Fly behavior interface and behavior implementation classes.
################################################################################
class FlyBehavior():
    def __init__(self):
        pass

class Fly(FlyBehavior):
    def fly(self):
        print "I'm flying!!"

class FlyNoWay(FlyBehavior):
    def fly(self):
        print "I can't fly"

class FlyRocketPowered(FlyBehavior):
    def fly(self):
        print "I'm flying with a rocket!"

################################################################################
# Quack behavior interface and behavior implementation classes.
################################################################################
class QuackBehavior():
    def __init__(self):
        pass

class Quack(QuackBehavior):
    def quack(self):
        print "Quack"

class MuteQuack(QuackBehavior):
    def quack(self):
        print "<< Silence >>"

class SqueakQuack(QuackBehavior):
    def quack(self):
        print "Squeak"

################################################################################
# Test Code.
################################################################################
if __name__ == "__main__":

    print '#'*80
    print "Mallard Duck"
    print '#'*80
    mallard = MallardDuck()
    mallard.perform_quack()
    mallard.perform_fly()

    print '#'*80
    print "Model Duck"
    print '#'*80
    model = ModelDuck()
    model.perform_fly()
    fly_rocket_powered_instance = FlyRocketPowered()
    model.set_fly_behavior(fly_rocket_powered_instance)
    model.perform_fly()

    print '#'*80
    print "Decoy Duck"
    print '#'*80
    decoy = DecoyDuck()
    decoy.perform_fly()
    decoy.perform_quack()

The devil’s in the _details

Your suggestion is good, but you don’t need a property; you can just use a normal attribute. All your setter does is set the variable, so just do that instead. So, this:

self.set_fly_behavior(fly_instance)

Can become this:

self._fly_behaviour = fly_instance

Note my use of self._fly_behaviour. Since it’s is an implementation detail of the class, the best practice is to prefix the variable name with an underscore. This tells programmers that it’s not designed to be used externally.

Aside: naming behaviours

The names perform_quack() and perform_fly() are very un-Pythonic. Why not just quack() and fly()? The perform tells us nothing—except, maybe leaking an implementation detail to the outside (that these delegated). This isn't something a caller needs to care about.

ABCs

Other than that, you could make your base classes Abstract Base Classes:

from abc import ABCMeta, abstractmethod

...

class QuackBehavior(object):
    __metaclass__ = ABCMeta

    @abstractmethod
    def quack(self):
        pass

    @classmethod
    def __subclasshook__(cls, Check):
        required = ["quack"]
        rtn = True
        for r in required:
          if not any(r in vars(BaseClass) for BaseClass in Check.__mro__):
            rtn = NotImplemented
        return rtn

This allows you to have a more Pythonic interface for your class.

(Also, note the removal of the empty constructor. Python provides an empty constructor anyway, so don’t bother defining one if you do nothing with it. It’s just useless extra code.)

A functional approach

This is a pretty heavyweight solution for Python. If you know that these behaviours are always just one function, I would argue classes aren’t needed.

Instead, recall that Python functions are first-class objects:

def quack(self):
    print("Quack")

def mute_quack(self):
    print "<< Silence >>"

some_duck = Duck()
some_duck.quack = mute_quack

A hybrid approach

We can make this work with one small modification: by using a property to convert functions into methods, so that they can be called normally.

(I’m assuming you need access to self in the method; if not, leave out that argument, and it’ll work as-is, like a static method.)

import types

class Duck(object):
    ...

    @property
    def fly(self):
        return self._fly

    @fly.setter
    def fly(self, value):
        self._fly = types.MethodType(value, self)

    @property
    def quack(self):
        return self._quack

    @quack.setter
    def quack(self, value):
        self._quack = types.MethodType(value, self)

One step further...

Hmm...we’re doing the same thing for both properties.  If you want, we can take it a step further.

Let’s generalise it instead:

import types

def method_property(name):
    def getter(self):
        return getattr(self, name)
    def setter(self, value):
        setattr(self, name, types.MethodType(value, self))
    return getter, setter

class Duck(object):
    ...
    quack = property(*method_property("_quack"))
    fly = property(*method_property("_fly"))

Just assign the function as needed—perhaps in the constructor of subclasses. Obviously, if you need more complicated behaviours, this might not do. But for simple things, I’d say it’s the better solution.

Constructors for subclassing

Since a duck is useless without a quack and fly behaviour, and all subclasses assign them, it makes sense for the constructor to deal with this:

class Duck(object):
    def __init__(self, fly, quack):
        self.fly = fly
        self.quack = quack

    ...

class ModelDuck(Duck):
    def __init__(self):
        Duck.__init__(self, fly_no_way, squeak_quack)
        print "I'm a Model Duck"

Or, if you were using your original class’s implementation:

class Duck(object):
    def __init__(self, fly, quack):
        self._fly_behaviour = fly
        self._quack_behaviour = quack

    ...

class ModelDuck(Duck):
    def __init__(self):
        Duck.__init__(self, FlyNoWay(), SqueakQuack())
        print "I'm a Model Duck"

Conclusion

If we wrap all this together, we end up with two implementations:

function based, and
class based.

These implementations remove repetition, and will make this sort of thing much easier to work with in Python overall.


"Algorithmic Crush" problem hitting timeout errors This is the HackerRank problem Algorithmic Crush:

  You are given a list of size N, initialized with zeroes. You have to perform M operations on the list and output the maximum of final values of all the N elements in the list. For every operation, you are given three integers a, b and k. The value k needs to be added to all the elements ranging from index a through b (both inclusive).

  Input Format

  The first line will contain two integers N and M separated by a single space.
  The next M lines will each contain three integers a, b and k separated spaces.
  The numbers in the list are numbered from 1 to N.

  Output Format

  A single integer on a separate line line containing maximum value in the updated list.

  Constraints

  \$3 ≤ N ≤ 10^7\$
    \$1 ≤ M ≤ 2 \cdot 10^5\$
    \$1 ≤ a ≤ b ≤ N\$
    \$0 ≤ k ≤ 10^9\$

  Sample input:

5 3
1 2 100
2 5 100
3 4 100

  Sample output:

200

  Explanation

  After the first update, the list will be 100 100 0 0 0.
  After the second update, the list will be 100 200 100 100 100.
  After the third update, the list will be 100 200 200 200 100.
  So, the required answer will be 200.

This is my Python 3 solution:

tempArray = [int(n) for n in input().split(" ")]
N = tempArray[0]
M = tempArray[1]
arr = [0 for x in range(N)]
for i in range(0,M):
    tempArray = [int(n) for n in input().split(" ")]
    for j in range(tempArray[0]-1, tempArray[1]):
        arr[j] = arr[j] + tempArray[2]
print(max(arr))

It gives me running time-out error for some test cases. I know very little about time complexity but I think it's \$O(NM)\$. What am I doing wrong?

Constraints

  \$3 ≤ N ≤ 10^7\$
    \$1 ≤ M ≤ 2 \cdot 10^5\$
    \$1 ≤ a ≤ b ≤ N\$
    \$0 ≤ k ≤ 10^9\$

You should verify that every value you read from the user is a valid value in your program.

As far as complexity goes, a suffix tree with a complexity of \$O(N\log{}N)\$ compared to your current \$O(MN)\$ solution that adds difference to every value in the range \$[A,B]\$.  With \$N \sim 10^7\$, an \$O(N\log{}N)\$ solution may still be too slow.

For this problem, I would employ a max prefix sum scan on a difference array.  To generate a set of range differences, we adjust \$arr_{a} \mathrel{{+}{=}} k\$ and \$arr_{b+1} \mathrel{{-}{=}} k\$.  Going back to your sample data, your difference array would look like this:

         #   1.......................N...N+1
5 3      #   0     0     0     0     0     0
1 2 100  # 100     0  -100     0     0     0
2 5 100  # 100   100  -100     0     0  -100
3 4 100  # 100   100     0     0  -100  -100

To calculate the max prefix sum, accumulate the difference array to \$N\$ while taking the maximum accumulated prefix. (*) denotes new max found.

DiffArray  100  100    0    0  -100  -100
PrefixSum  100*
Max = 100

DiffArray  100  100    0    0  -100  -100
PrefixSum       200*
Max = 200

DiffArray  100  100    0    0  -100  -100
PrefixSum            200
Max = 200

DiffArray  100  100    0    0  -100  -100
PrefixSum                 200
Max = 200

DiffArray  100  100    0    0  -100  -100
PrefixSum                       100
Max = 200

Building a difference array in \$O(M)\$ and finding the max prefix sum in \$O(N)\$ results in a \$O(M+N)\$ linear algorithm.


Atomic floating-point addition I need to be able to atomically add a 32-bit floating point value to a location in memory. Here is what I came up with. While the code is Windows-specific, I'll extend it with Linux support using __sync_bool_compare_and_swap().

While I tested this code and it appeared to work fine, I'll appreciate a fresh look from another pair of eyes to make sure this code is 100% safe.

I'm also very much interested in any performance advice as this code is used in a hot code path in a performance-sensitive application (interactive rendering).

atomic_float_add(), the main function:

__forceinline void atomic_float_add(volatile float* ptr, const float operand)
{
    assert(is_aligned(ptr, 4));

    volatile LONG* lptr = reinterpret_cast<volatile LONG*>(ptr);
    LONG lorg, lnew;

    do
    {
        const float forg = *ptr;
        const float fnew = forg + operand;
        lorg = binary_cast<LONG>(forg);
        lnew = binary_cast<LONG>(fnew);
    } while (InterlockedCompareExchange(lptr, lnew, lorg) != lorg);
}

binary_cast() is implemented in such a way that it obeys strict aliasing rules:

template <typename Target, typename Source>
inline Target binary_cast(Source s)
{
    BOOST_STATIC_ASSERT(sizeof(Target) == sizeof(Source));

    union
    {
        Source  m_source;
        Target  m_target;
    } u;

    u.m_source = s;
    return u.m_target;
}

is_aligned():

template <typename T>
inline bool is_aligned(const T ptr, const size_t alignment)
{
    assert(alignment > 0);
    assert(is_pow2(alignment));

    const uintptr_t p = (uintptr_t)ptr;
    return (p & (alignment - 1)) == 0;
}

is_pow2():

template <typename T>
inline bool is_pow2(const T x)
{
    return (x & (x - 1)) == 0;
}

Assembly output for atomic_float_add() (Visual Studio 2013, full optimization):

lorg$ = 8
ptr$ = 8
lnew$ = 16
operand$ = 16
?atomic_float_add@?A0x1534477b@foundation@@YAXPECMM@Z PROC

    prefetchw BYTE PTR [rcx]
    npad    13

$LL3@atomic_flo:
    movss   xmm0, DWORD PTR [rcx]
    movss   DWORD PTR lorg$[rsp], xmm0
    addss   xmm0, xmm1
    mov eax, DWORD PTR lorg$[rsp]
    movss   DWORD PTR lnew$[rsp], xmm0
    mov edx, DWORD PTR lnew$[rsp]
    lock cmpxchg DWORD PTR [rcx], edx
    jne SHORT $LL3@atomic_flo

    ret 0

?atomic_float_add@?A0x1534477b@foundation@@YAXPECMM@Z ENDP

EDIT: here's a bit of context on how this code is used:

As you can see, atomic_float_add() is taking the bulk of the time of FilteredTile::add(), which itself is the most expensive function of the whole program in this particular setup, taking nearly 27% of the overall time.

@EOF commented that InterlockedCompareExchange (and its gcc counterpart, __sync_val_compare_and_swap) returns the initial value of the destination address. This allows to remove one memory load from the retry loop.

Here is the new version with this optimization:

__forceinline void atomic_float_add(volatile float* ptr, const float operand)
{
    assert(is_aligned(ptr, 4));

    volatile LONG* iptr = reinterpret_cast<volatile LONG*>(ptr);
    LONG expected = *iptr;

    while (true)
    {
        const float value = binary_cast<float>(expected);
        const LONG new_value = binary_cast<LONG>(value + operand);
        const LONG actual = InterlockedCompareExchange(iptr, new_value, expected);
        if (actual == expected)
            return;
        expected = actual;
    }
}

Here is the corresponding assembly:

?atomic_float_add@foundation@@YAXPECMM@Z PROC

    mov eax, DWORD PTR [rcx]
    mov r8, rcx
    mov DWORD PTR value$2[rsp], eax
    movss   xmm0, DWORD PTR value$2[rsp]
    addss   xmm0, xmm1
    movss   DWORD PTR new_value$1[rsp], xmm0
    mov edx, DWORD PTR new_value$1[rsp]
    lock cmpxchg DWORD PTR [rcx], edx
    je  SHORT $LN16@atomic_float_add
    npad    13

$LL3@atomic_float_add:
    mov DWORD PTR value$2[rsp], eax
    mov edx, eax
    movss   xmm0, DWORD PTR value$2[rsp]
    addss   xmm0, xmm1
    movss   DWORD PTR new_value$1[rsp], xmm0
    mov ecx, DWORD PTR new_value$1[rsp]
    lock cmpxchg DWORD PTR [r8], ecx
    cmp eax, edx
    jne SHORT $LL3@atomic_float_add

$LN16@atomic_float_add:
    ret 0

?atomic_float_add@foundation@@YAXPECMM@Z ENDP

Interestingly, the compiler decided to do one iteration before entering the loop. I'm not sure I understand the benefit of this...


N closest points to the reference point Here is working code to get N closest points to some reference point.

Please help to improve it, specifically by commenting on my use of std algorithms and vectors/iterators manipulations.

Nearest.h

#pragma once

#include <iostream>
#include <algorithm>
#include <cmath>
#include <vector>

namespace nearest {

struct Point {
  double x;
  double y;
  double z;

  bool operator==(const Point& rhp) {
    return x == rhp.x && y == rhp.y && z == rhp.z;
  }

};

// Return N closest points to first point in vector in order
std::vector<Point> nearestN(const std::vector<Point>& points,
                            int N);

// Return N closest points to a reference point, in order - all
// within maximum distance threshold
std::vector<Point> nearestN(const std::vector<Point>& points,
                            int N,
                            const Point& reference,
                            double distanceThreshold);

}

Nearest.cpp

#include "Nearest.h"

using namespace std;

namespace nearest {

    // to print Point to cout
    std::ostream& operator<<(std::ostream &o, const nearest::Point& p) {
        o << "(" << p.x << ", " << p.y << ", " << p.z << ")" << std::endl;
        return o;
    }

    // defines sorting order for points
    struct ByDistance {
        const Point& origin;

        bool operator()(const Point& lp, const Point& rp) {
            return dist(origin, lp) < dist(origin, rp);
        }

        static double dist(const Point& lp, const Point& rp){
            double xs = (lp.x - rp.x) * (lp.x - rp.x);
            double ys = (lp.y - rp.y) * (lp.y - rp.y);
            double zs = (lp.z - rp.z) * (lp.z - rp.z);
            return sqrt(xs + ys + zs);
        }
    };

    vector<Point> nearestN(const vector<Point>& points, int N) {
        if (points.size() == 0) {
            vector<Point> empty;
            return empty;
        }
        return nearestN(points, N, points[0], INFINITY);
    }

    vector<Point> nearestN(const vector<Point>& points,
                           int N,
                           const Point& reference,
                           double distanceThreshold) {

        vector<Point> temp;
        temp.insert(temp.begin(), points.begin(), points.end());

        // filtering vector to remove all points far then distance from reference
        temp.erase(remove_if(temp.begin(),
                             temp.end(),
                             [&reference, distanceThreshold](Point& p){ return ByDistance::dist(p, reference) > distanceThreshold; }),
                   temp.end());

        ByDistance distance = {reference};
        sort(temp.begin(), temp.end(), distance);

        auto sz = min(static_cast<int>(temp.size()), N);
        vector<Point> result(sz);
        copy_n(temp.begin(), sz, result.begin());
        return result;
    }
}

Faster sorting
If the only purpose of the distance computation is for comparison, then you can speed it up by removing the call to sqrt().  You'll end up comparing the squares of the distances, which is equivalent to comparing the actual distances.


Image resizing in Rust Are there Rust features I could apply to optimize for simple test of a JPEG or PNG resized from 2000 x 2000 pixels to 150 x 150 pixels?

extern crate image;
use std::env;

fn main() {
    let mut args = env::args();
    args.next();
    let file_location = args.next().unwrap();
    let width = args.next().unwrap().parse().unwrap();
    let height = args.next().unwrap().parse().unwrap();

    let img = image::open(file_location.as_str()).unwrap();

    // img.resize(width, height, image::imageops::Lanczos3);
    img.thumbnail(width, height);
}

I would slightly change this code:

use std::error::Error;

fn main() -> Result<(), Box<dyn Error>> {
    // Args arrangement
    let mut args = std::env::args().skip(1);
    assert_eq!(args.len(), 3, "Arguments must be: file_location width height");

    // Reading args
    let file_location = args.next().unwrap();
    let width = args.next().unwrap().parse()?;
    let height = args.next().unwrap().parse()?;

    // Do the job
    let img = image::open(&file_location)?;
    img.thumbnail(width, height);

    // All was ok
    Ok(())
}

You can take advantage of the try operator ? to make the error handling easier.
Similarly, you can add a bit of error handling with the assert_eq line. You can use the len method because this iterator knows exactly how much elements it has.
You can use skip to discard the first element of the Args iterator.