Project Euler Problem 21 in Python: Summing amicable numbers Let d(n) be defined as the sum of proper divisors of n (numbers less than n which divide evenly into n).
  If d(a) = b and d(b) = a, where a ≠ b, then a and b are an amicable pair and each of a and b are called amicable numbers.

  For example, the proper divisors of 220 are 1, 2, 4, 5, 10, 11, 20, 22, 44, 55 and 110; therefore d(220) = 284. The proper divisors of 284 are 1, 2, 4, 71 and 142; so d(284) = 220.

  Evaluate the sum of all the amicable numbers under 10000.

My solution to the 21 problem on project Euler is very slow (with pure brute force it took 30 mins to find the solution). Any ways that I can improve my code or should I scrap it and think about something else ?

Here's my code :

import time, math

start = time.time()

amicable_nums = set()

def sum_div(n):
    divisors = []
    for x in range(1, int(math.sqrt(n) + 1)):
        if n % x == 0:
            divisors.append(x)

            if x * x != n and x != 1:
                divisors.append(int(n / x))
    return sum(divisors)

for i in range(1, 10000):
    for j in range(1, 10000):
        if sum_div(i) == j and sum_div(j) == i and i != j:
            amicable_nums.update([i, j])

print(sum(amicable_nums))
print("It took " + str(time.time() - start) + " seconds")

In your sum_div method, 1 is always a divisor, and n/1 is never a “proper” divisor of n.  So you can remove the “1” case from your loop, and get rid of the x != 1 test, for a slight speed increase.

Also, if n % x == 0, then the other divisor is n // x; no call to int() is required.

Finally, no list creation is required.  Just maintain a running total.

def sum_div(n):
    total = 1
    for x in range(2, int(math.sqrt(n) + 1)):
        if n % x == 0:
            total += x
            y = n // x
            if y > x:
                total += y
    return total

In your main loop, your search for i,j both in range(1,10000), and add both i, j (if different) to your set to ensure you don’t have duplicates.

You can cut down your search space by half by recognizing that if you find one pair i, j, you have also found the other pair j, i, and adding the other pair to the set is a no-op.  You only need to search the cases where (say) i < j is true, which eliminates just over half of the pairs.

for i in range(1, 10000):
    for j in range(i+1, 10000):
        if sum_div(i) == j and sum_div(j) == i:
            amicable_nums.update([i, j])

How many times are you computing sum_div(i)???  Does it change in the inner loop?  Maybe you could only compute it once per loop?

for i in range(1, 10000):
    sdi = sum_div(i)
    for j in range(i+1, 10000):
        if sdi == j and sum_div(j) == i:
            amicable_nums.update([i, j])

Do you really need the inner loop?  sdi == j will only be true for exactly one value in the loop.


Create news ticker animation I have this function in this fiddle

JavaScript:

var newsTicker = function (ele) {
    var eles = ele.find('ul li'),
        indexEle = 0,
        dataEle = ele.find('.ticker-post'),
        rotateChars = function (title) {
            dataEle.text('');
            indexCut = 0;
            rotateCharsTimer = setInterval(function () {
                if (dataEle.text().length == title.length) {
                    clearInterval(rotateCharsTimer);
                } else if (dataEle.text().length < title.length) {
                    text = dataEle.text().concat(title.substr(indexCut, 1));
                    dataEle.text(text);
                };
                if (title.length - 1 > indexCut) indexCut++;
                else indexCut = 0;
            }, 90);
        },
        loopLs = function () {
            loopEle = eles.get(indexEle);
            eleHref = $(loopEle).data('href');
            eleTitle = $(loopEle).data('title');
            dataEle.attr('href', eleHref);
            if (typeof rotateCharsTimer != 'undefined') {
                dataEle.fadeOut();
                clearInterval(rotateCharsTimer);
            }
            dataEle.fadeIn();
            rotateChars(eleTitle);
            if (eles.length - 1 > indexEle) indexEle++;
            else indexEle = 0;
        }
    loopLs();
    setInterval(function (){loopLs()},9400);
}

HTML:

Breaking News:

Is this the best way to do this?

Your jQuery function was okay, but it's common practice now to put your animations into CSS wherever possible.

I've rewritten your code, moving as much as possible into CSS. I'll explain how and why below, or you can skip to the final solution here.

Semantically your links shouldn't be data-attributes as they control page navigation and need to be visible and identifiable as such.

Breaking News:

            No 10 armed police arrested over hardcore pornography

            EDF extends life of UK nuclear plants

You shouldn't use float: left for this, these components are inline, not floating.

#news-ticker .ticker-title {
    display: inline-block;
    margin-right: 12px;
}

#news-ticker ul {
    display: inline-block;
    position: relative;
}

Next we want to make each news item invisible when not active, and we want them to all appear in the same place. A simple way to do this is to make their position absolute.

#news-ticker li {
    position: absolute;
    left: 0;
    width: 0;
    overflow: hidden;
    height: 1em;
    word-wrap: break-word;
    opacity: 0
}

There are a few more things I've added here as well...

width: 0: By making the width of the elements 0 they are invisible, and we can extend the width to show them a letter at a time.
word-wrap: break-word: This helps us get the tick each letter at a time effect.
overflow: hidden: stops us from seeing the rest of the sentence when the container isn't wide enough.

Next we make the animation. There's a good guide here about how they work.

#news-ticker li.tick {
    -webkit-animation: tick 5s linear;

}

@-webkit-keyframes tick {
  0% {
    width: 0;
  }
  5% {
      opacity: 1;
  }
  90% {
    width: 550px;
    opacity: 1;
  }
  100% {
     opacity: 0
  }
}

This means that when we add the class .tick to one of our list elements it will animate from 0 width to 550px.

Now for our small amount of JavaScript to switch the classes and make the wrap work on letters instead of words.

$(function () {
   var $ticker = $('#news-ticker'),
   $first = $('li:first-child', $ticker);

Unfortunately there's no letter-wrap CSS attribute, so we have to cheat a little. Here I insert a blank space inbetween each letter, essentially making it a word.

   $('a', $ticker).each(function () {
       var $this = $(this),
         text = $this.text();
       $this.html(text.split('').join('&#8203;'));
   });

And then start the ticker. Every time an animation ends, it grabs the next (or first) element and continues:

   function tick($el) {
     $el.addClass('tick')
       .one('webkitAnimationEnd oanimationend msAnimationEnd animationend', function () {

         $el.removeClass('tick');
           var $next = $el.next('li');
           $next = $next.length > 0 ? $next : $first;
         tick($next);
     });
   }

   tick($first);

});


FizzBuzz for CodeEval I'm a beginner programmer and decided to try some coding challenges. I found CodeEval and attempted the first challenge, FizzBuzz. However upon submitting my code I found that my submission only partially solved the problem.

Link to the challenge

  Challenge Description:

  Players generally sit in a circle. The first player says the number
  “1”, and each player says next number in turn. However, any number
  divisible by X (for example, three) is replaced by the word fizz, and
  any divisible by Y (for example, five) by the word buzz. Numbers
  divisible by both become fizz buzz. A player who hesitates, or makes a
  mistake is eliminated from the game.

  Write a program that prints out the final series of numbers where
  those divisible by X, Y and both are replaced by “F” for fizz, “B” for
  buzz and “FB” for fizz buzz.

  Input Sample:

  Your program should accept a file as its first argument. The file
  contains multiple separated lines; each line contains 3 numbers that
  are space delimited. The first number is the first divider (X), the
  second number is the second divider (Y), and the third number is how
  far you should count (N). You may assume that the input file is
  formatted correctly and the numbers are valid positive integers.

  For Example:

  1) 3 5 10

  2) 2 7 15

  Output Sample:

  Print out the series 1 through N replacing numbers divisible by X with
  “F”, numbers divisible by Y with “B” and numbers divisible by both
  with “FB”. Since the input file contains multiple sets of values, your
  output should print out one line per set. Ensure that there are no
  trailing empty spaces in each line you print.

  1) 1 2 F 4 B F 7 8 F B

  2) 1 F 3 F 5 F B F 9 F 11 F 13 FB 15

How can I improve my code speed-wise and memory-wise so that I can fully complete this challenge?

The code is working for my test cases.

import sys

def main(name_file):
    _test_cases = open(name_file, 'r')
    for test in _test_cases:
        result = ""
        if len(test) == 0:
            break
        else:
            test = test.split()
            first_div = int(test[0])
            second_div = int(test[1])
            lim = int(test[2])
            for i in range(1, lim+1):
                if i % (first_div*second_div) == 0:
                    result += "FB "
                elif i % first_div == 0:
                    result += "F "
                elif i % second_div == 0:
                    result += "B "
                else:
                    result += "%d " % i
            print(result[:-1])

    _test_cases.close()
if __name__ == '__main__':
    main(sys.argv[1])

There is a bug in your code for those times when the two input values are not prime.

Consider an instance when the input values are 4 and 6. In this case, your code will output F for multiples of 4, and B for multiples of 6, and FB for multiples of 24.... great... but, is it? No, 12 is a multiple of both, but will only print "F ".

If you want to use the optimized cascading-if-else version of FizzBuzz, then you need to ensure the inputs are both prime, or have no common factors.

It would often still be faster to compute the common factors and then after that do the system you have.


Testing individual nodes launched by naught using supertest The purpose: an express node.js server which can be launched using naught, with an integration test using supertest. Each worker naught runs will test itself on a unique port, and only if passes it will listen in on the main port and join the pool.

Major concerns are - no idea what I'm doing.  How I provision the ports for the test nodes, closing the test servers (need to clear up memory)?

This just a POC, structuring is required for a production app.

var testport = 10000 + process.pid;
var server = app.listen(testport, function() {
    console.log('pid %d listening on port %d ...', process.pid, testport);

    supertest(server)
    .get('/test')
    .expect(200)
    .end(function(err, res){

        if (err) {
            console.log(err);
            if (process.send) process.send('offline');
            process.exit(-1);
        }
        else {
            console.log('pid %d tested OK, closing test server and launching main one...', process.pid);
            server.close();
            listenToMainport();
        }

    });
});

You can see the full context of the example here.

I think you should write more code before submitting, the fun part to review is the code that you wrote after the prototype ;)

From a quick once over:

JsHint finds no fault except for some mixed use of tabs and space
Consider 'use strict', it can save lives
Consider something better than console.log, especially if you are going to use this code for performance testing
Use lowerCamelCase consistently: testport -> testPort


Four fours to get any number I decided to solve a problem from Code Golf called the four fours.
Problem description:

The Four fours puzzle is a popular recreational mathematical puzzle
that involves using exactly four 4s (and no other number) and a
defined set of operations to reach every number from 0 to a given
maximum.
In this version, the only following operators are allowed:

Any grouping symbols may be used
Addition (+), Subtraction (-), Multiplication (*), Division (/)
Factorial (!), Gamma function (Γ)
Exponentiation (^), Square root (√)
*Concatenation (eg. 44 is two 4s)
Decimal point (eg. 4.4 is two 4s), Overbar (eg. .4~ = 4/9)

Some assumption/changes I made:

I assumed that squaring a number is ok (using 4**2 or 16)
I assumed that decimal values, in the cases where they occur can be truncated (4.9 = 4)

Description of the algorithm
First I create a list of all the possible start values, that is:

The number itself
The number squared
The sqrt of the number
The factorial of the number
The gamma function of the number (factorial(number -1))

Then I iterate throu them with the random_reduce function that 'reduces' the list by applying each time a random function.
The function takes about 2 secons for all numbers upto a 100, finding a way to create all the numbers along the way.
I am looking for code-style advice and maybe a better algorithm suggestion.
from __future__ import division

import random
import operator as op
import math

def get_starts(lst,n):
    sqrt,power,fact,gamma = int(n**0.5),n**2,math.factorial(n),math.factorial(n-1)
    return ( [a,b,c,d] for a in (sqrt,n,power,fact,gamma) \
             for b in (sqrt,n,power,fact,gamma) for c in (sqrt,n,power,fact,gamma) \
             for d in (sqrt,n,power,fact,gamma))

def random_reduce(lst,functions):
    functions_used = []
    result = lst[0]
    for i in lst[1:]:
        fun = random.choice(functions)
        functions_used.append(fun)
        result = fun(result,i)
    return result,[i.__name__ for i in functions_used]

def how_to_obtain(n,lst,functions):
    for i in range(500):
        for l in get_starts(lst,4):
            result = random_reduce(l,functions)
            if n == result[0]:
                return n,[l,result[1]]

def concat(a,b):
    return int(str(a) + str(b))

def div(a,b):
    return a // b

def solve_44(max_):
    numbers_solutions = {}
    for i in range(max_):
        numbers_solutions[i] = how_to_obtain(i,[4,4,4,4],[concat, op.add, op.mul, op.sub,
                                         div, op.pow, op.mod])

    return numbers_solutions

def ratio_of_Nones_in_dictionary(dict_):
    return (len({i:dict_[i] for i in dict_ if dict_[i] == None}) / len(dict_))

def main(max_):
    solution = solve_44(max_)
    for i in solution:
        print(solution[i])
    print("The random search failed to find a solution {}% of the times.".format(
        ratio_of_Nones_in_dictionary(solution)*100))

if __name__ == "__main__":
    main(100)

I am looking for code-style advice and maybe a better algorithm suggestion.

As for coding style, first and foremost, please follow PEP8.

Simplify

You could simplify get_starts using product from itertools:

def get_starts(n):
    sqrt, power, fact, gamma = int(n ** 0.5), n ** 2, math.factorial(n), math.factorial(n - 1)
    return product((sqrt, n, power, fact, gamma), repeat=4)

I also dropped the unused lst parameter.
In fact many of the functions have unused parameters,
remove them everywhere.

Use generators, they are awesome

Since you accumulate solutions in an array,
the program seems to freeze for a few seconds while calculating,
rather than printing the solutions that are ready.
You could improve that by using a generator instead, for example:

def solve_44(max_):
    for i in range(max_):
        yield how_to_obtain(i, [concat, op.add, op.mul, op.sub, div, op.pow, op.mod])

def main(max_):
    none_count = 0
    for solution in solve_44(max_):
        print(solution)
        if solution is None:
            none_count += 1
    print("The random search failed to find a solution {}% of the times.".format(
        none_count / max_ * 100))

Another correction here is using is None instead of == None for comparing with None values.


Alarm clock subclass of a clock I have these two classes and they should remain separate classes, but I have a feeling that this could be implemented more efficiently and with less duplicate code. After googling around I still didn't figure a better way, so perhaps someone here could point me out. Ideally I want to get tick() method from Clock and just add alarm to it and also get set_time() method with additional attributes for setting the alarm time.
class Clock:

def __init__(self, hours, minutes, seconds):
    self.set_time(hours, minutes, seconds)

def set_time(self, hours, minutes, seconds):
    if hours < 0:
        self.hours = 0
    elif hours > 23:
        self.hours = 23
    else:
        self.hours = hours

    if minutes < 0:
        self.minutes = 0
    elif minutes > 59:
        self.minutes = 59
    else:
        self.minutes = minutes

    if seconds < 0:
        self.seconds = 0
    elif seconds > 59:
        self.seconds = 59
    else:
        self.seconds = seconds

def tick(self):
    while True:
        print(str(self.hours).zfill(2), ":", str(self.minutes).zfill(2), ":", str(self.seconds).zfill(2), sep="")
        self.seconds += 1
        time.sleep(1)
        if self.seconds == 60:
            self.seconds = 0
            self.minutes += 1
            if self.minutes == 60:
                self.minutes = 0
                self.hours += 1
                if self.hours == 24:
                    self.hours = 0

class AlarmClock(Clock):

def __init__(self, hours, minutes, seconds, alarm_hr, alarm_min, alarm_sec):
    super().__init__(hours, minutes, seconds)
    self.set_alarm(alarm_hr, alarm_min, alarm_sec)

def set_alarm(self, alarm_hr, alarm_min, alarm_sec):
    if alarm_hr < 0:
        self.alarm_hr = 0
    elif alarm_hr > 23:
        self.alarm_hr = 23
    else:
        self.alarm_hr = alarm_hr

    if alarm_min < 0:
        self.alarm_min = 0
    elif alarm_min > 59:
        self.alarm_min = 59
    else:
        self.alarm_min = alarm_min

    if alarm_sec < 0:
        self.alarm_sec = 0
    elif alarm_sec > 59:
        self.alarm_sec = 59
    else:
        self.alarm_sec = alarm_sec

def alarm(self):
    while True:
        print(str(self.hours).zfill(2), ":", str(self.minutes).zfill(2), ":", str(self.seconds).zfill(2), sep="")
        self.seconds += 1
        time.sleep(1)
        if self.seconds == 60:
            self.seconds = 0
            self.minutes += 1
            if self.minutes == 60:
                self.minutes = 0
                self.hours += 1
                if self.hours == 24:
                    self.hours = 0
        if self.hours == self.alarm_hr and self.minutes == self.alarm_min and self.seconds == self.alarm_sec:
            print("ALARM ALARM ALARM ALARM ALARM!!!!")
            break

I have refactored some of your code, please note that I am ignoring pre-existing libraries since I assume there's a particular reason (educational or otherwise) behind implementing Clock, tick, etc. this way on your own.
class Clock:
class Clock:
    def __init__(self, hours: int, minutes: int, seconds: int):
        self.hours = self.normalize_time_in_range(hours, 0, 23)
        self.minutes = self.normalize_time_in_range(minutes, 0, 59)
        self.seconds = self.normalize_time_in_range(seconds, 0, 59)

    @staticmethod
    def normalize_time_in_range(value: int, lower_bound: int, upper_bound: int) -> int:
        if value < lower_bound:
            return lower_bound
        elif value > upper_bound:
            return upper_bound
        else:
            return value

    def start_ticking(self):
        while True:
            self.tick()

    def tick(self):
        print(f"{str(self.hours).zfill(2)}:{str(self.minutes).zfill(2)}:{str(self.seconds).zfill(2)}")
        self.seconds += 1
        time.sleep(1)
        if self.seconds == 60:
            self.seconds = 0
            self.minutes += 1
            if self.minutes == 60:
                self.minutes = 0
                self.hours += 1
                if self.hours == 24:
                    self.hours = 0

normalize_value_in_range: Notice you have a lot of duplicated logic in set_time and set_alarm. Most of the logic can be extracted to this more generic implementation.
start_ticking and tick: In your case I found it useful to seperate the loop from the actual tick logic so you can change the loop conditions in subclasses while reusing tick. You will see why this is useful in AlarmClock.
Type annotations: As you can see in the method heads I added some type hints to the newly added methods. This is generally a good idea in regards to documenting your code for yourself and others.

class AlarmClock:
class AlarmClock(Clock):
    def __init__(self, hours, minutes, seconds, alarm_hr, alarm_min, alarm_sec):
        super().__init__(hours, minutes, seconds)

        self.alarm_hr = self.normalize_value_in_range(alarm_hr, 0, 23)
        self.alarm_min = self.normalize_value_in_range(alarm_min, 0, 59)
        self.alarm_sec = self.normalize_value_in_range(alarm_sec, 0, 59)

        self.ticking = False

    def start_ticking(self):
        self.ticking = True
        while self.ticking:
            self.tick()

    def tick(self):
        super().tick()
        if self.hours == self.alarm_hr and self.minutes == self.alarm_min and self.seconds == self.alarm_sec:
            print("ALARM ALARM ALARM ALARM ALARM!!!!")
            self.ticking = False

normalize_value_in_range: We can now reuse this method to set our alarm time as well.
start_ticking and tick: Since we seperated the loop from the actual tick we can use Clock.tick() in our AlarmClock without the need for duplicate code. Adding the attribute self.ticking to AlarmClock allows us to break out of the loop across methods. The reason we could not reuse tick() before was because it contained the infinite while True loop, thereby not allowing us to intercept when the alarm_time is reached.

class Time:
Lastly, I would suggest seperating time logic from clock logic. This makes the clocks more concise and allows you to extend the functionality of the Time class as needed. You could also say it allows the clocks to focus on their main functionality. Again, I am ignoring the existence of libraries for this task, since I presume you want to implement the entire logic on your own. Specialized libraries like datetime are worth looking into since they provide a lot of comfortable functionality. One implementation might be the following:
class Time:
    def __init__(self, hours: int, minutes: int, seconds: int):
        self.hours = self.normalize_value_in_range(hours, 0, 23)
        self.minutes = self.normalize_value_in_range(minutes, 0, 59)
        self.seconds = self.normalize_value_in_range(seconds, 0, 59)

    @staticmethod
    def normalize_value_in_range(value: int, lower_bound: int, upper_bound: int) -> int:
        if value < lower_bound:
            return lower_bound
        elif value > upper_bound:
            return upper_bound
        else:
            return value

    def increment(self):
        self.seconds += 1
        if self.seconds == 60:
            self.seconds = 0
            self.minutes += 1
            if self.minutes == 60:
                self.minutes = 0
                self.hours += 1
                if self.hours == 24:
                    self.hours = 0

    def __eq__(self, other):
        return self.hours == other.hours and self.minutes == other.minutes and self.seconds == other.seconds

    def __str__(self):
        return f"{str(self.hours).zfill(2)}:{str(self.minutes).zfill(2)}:{str(self.seconds).zfill(2)}"

class Clock:
    def __init__(self, hours: int, minutes: int, seconds: int):
        self.time = Time(hours, minutes, seconds)

    def start_ticking(self):
        while True:
            self.tick()

    def tick(self):
        print(self.time)
        self.time.increment()
        time.sleep(1)

class AlarmClock(Clock):
    def __init__(self, hours, minutes, seconds, alarm_hr, alarm_min, alarm_sec):
        super().__init__(hours, minutes, seconds)
        self.alarm_time = Time(alarm_hr, alarm_min, alarm_sec)

        self.ticking = False

    def start_ticking(self):
        self.ticking = True
        while self.ticking:
            self.tick()

    def tick(self):
        super().tick()
        if self.time == self.alarm_time:
            print("ALARM ALARM ALARM ALARM ALARM!!!!")
            self.ticking = False


Excel VBA code that displays a 14 day weather forecast I am trying to see if anyone can help me in making this Excel VBA code a little faster in rendering the data from the weather site I am using.  Maybe instead of getting all 14 days it includes in the site, could I get help in getting just 10 days? Any help is much appreciated. Thanks!
Sub MiamiWeather()

   Dim HTTP As Object, HTML As Object, i As Integer, j As Integer

    Set HTML = CreateObject("HTMLFILE")
    Set HTTP = CreateObject("MSXML2.XMLHTTP")

    myURL = "https://weather.com/weather/tenday/l/3881cd527264bc7c99b6b541473c0085e75aa026b6bd99658c56ad9bb55bd96e"

    HTTP.Open "GET", myURL, False
    HTTP.send

    HTML.body.innerHTML = HTTP.responseText

    Set objCollection = HTML.getElementsByTagName("p")
    i = 0

    Do While i < objCollection.Length And j < 20
        If objCollection(i).getAttribute("data-testid") = "wxPhrase" Then
            j = j + 1
            Range("A" & j) = objCollection(i).PreviousSibling.PreviousSibling.FirstChild.innerText
            Range("B" & j) = objCollection(i).PreviousSibling.FirstChild.innerText

        End If
        i = i + 1
    Loop

    Set HTML = Nothing
    Set HTTP = Nothing
End Sub

The real question here is how to get only the p tags with a data-testid with the value of wxPhrase. This can be done using querySelectorAll().

Document.querySelectorAll("p[data-testid='wxPhrase']")

See Document.querySelectorAll().
You will need to add a reference to the Microsoft HTML Object Library and declare you document variable as HTMLDocument.

Dim Document As HTMLDocument

Loading the values into an Array will offer you the best performance.  It will not make a noticeable difference if you return 10 days or all the days of the month.
Refactored Code
Sub MiamiWeather()
    Dim Data As Variant
    Data = MiamiWeatherData

    Range("A1:B1").Value = Array("Date", "Tempature")
    Range("A2").Resize(UBound(Data), 2).Value = Data

End Sub

Function MiamiWeatherData()
    Const URL = "https://weather.com/weather/tenday/l/3881cd527264bc7c99b6b541473c0085e75aa026b6bd99658c56ad9bb55bd96e"
    Dim responseText As String
    With CreateObject("MSXML2.XMLHTTP")
        .Open "GET", URL, False
        .send
        responseText = .responseText
    End With

    Dim Document As HTMLDocument
    Set Document = CreateObject("HTMLFILE")
    Document.body.innerHTML = responseText

    Dim Children As IHTMLDOMChildrenCollection
    Set Children = Document.querySelectorAll("p[data-testid='wxPhrase']")
    Dim Results As Variant
    ReDim Results(1 To Children.Length, 1 To 2)
    Dim r As Long

    For r = 0 To Children.Length - 1
        Results(r + 1, 1) = Children(r).PreviousSibling.PreviousSibling.FirstChild.innerText
        Results(r + 1, 2) = Children(r).PreviousSibling.FirstChild.innerText
    Next

    MiamiWeatherData = Results
End Function


Simple Multithread Timer This is a very basic timer that can support multithreading with std::thread and std::chrono.

The timer has the classic functions: start() and stop().

The start() method creates an independent thread (if multithread support is enabled), then sleep the thread for a given Interval, then execute Timeout function. This method sets the running flag to true.

If singleShot flag is not enabled the sleepThenTimeout process is called while a running flag is true.

If multithread support is not enabled, the current thread is sleep.

The stop() method just sets the running flag to false and join the thread.

My doubt is about threat-safety.  I've just started to learn how multithreading works, so I'm not sure if I must use mutexes or something like that.

Any other type of feedback are welcome!

Timer.h

#ifndef TIMER_H
#define TIMER_H

#include <thread>
#include <chrono>

class Timer
{
public:
    typedef std::chrono::milliseconds Interval;
    typedef std::function<void(void)> Timeout;

    Timer(const Timeout &timeout);
    Timer(const Timeout &timeout,
          const Interval &interval,
          bool singleShot = true);

    void start(bool multiThread = false);
    void stop();

    bool running() const;

    void setSingleShot(bool singleShot);
    bool isSingleShot() const;

    void setInterval(const Interval &interval);
    const Interval &interval() const;

    void setTimeout(const Timeout &timeout);
    const Timeout &timeout() const;

private:
    std::thread _thread;

    bool _running = false;
    bool _isSingleShot = true;

    Interval _interval = Interval(0);
    Timeout _timeout = nullptr;

    void _temporize();
    void _sleepThenTimeout();
};

#endif // TIMER_H

Timer.cpp

#include "Timer.h"

Timer::Timer(const Timeout &timeout)
    : _timeout(timeout)
{
}

Timer::Timer(const Timer::Timeout &timeout,
             const Timer::Interval &interval,
             bool singleShot)
    : _isSingleShot(singleShot),
      _interval(interval),
      _timeout(timeout)
{
}

void Timer::start(bool multiThread)
{
    if (this->running() == true)
        return;

    _running = true;

    if (multiThread == true) {
        _thread = std::thread(
                    &Timer::_temporize, this);
    }
    else{
        this->_temporize();
    }
}

void Timer::stop()
{
    _running = false;
    _thread.join();
}

bool Timer::running() const
{
    return _running;
}

void Timer::setSingleShot(bool singleShot)
{
    if (this->running() == true)
       return;

    _isSingleShot = singleShot;
}

bool Timer::isSingleShot() const
{
    return _isSingleShot;
}

void Timer::setInterval(const Timer::Interval &interval)
{
    if (this->running() == true)
       return;

    _interval = interval;
}

const Timer::Interval &Timer::interval() const
{
    return _interval;
}

void Timer::setTimeout(const Timeout &timeout)
{
    if (this->running() == true)
       return;

    _timeout = timeout;
}

const Timer::Timeout &Timer::timeout() const
{
    return _timeout;
}

void Timer::_temporize()
{
    if (_isSingleShot == true) {
        this->_sleepThenTimeout();
    }
    else {
        while (this->running() == true) {
            this->_sleepThenTimeout();
        }
    }
}

void Timer::_sleepThenTimeout()
{
    std::this_thread::sleep_for(_interval);

    if (this->running() == true)
        this->timeout()();
}

main.cpp

#include <iostream>

#include "Timer.h"

using namespace std;

int main(void)
{
    Timer tHello([]()
    {
        cout << "Hello!" << endl;
    });

    tHello.setSingleShot(false);
    tHello.setInterval(Timer::Interval(1000));
    tHello.start(true);

    Timer tStop([&]()
    {
        tHello.stop();
    });

    tStop.setSingleShot(true);
    tStop.setInterval(Timer::Interval(3000));
    tStop.start();

    return 0;
}

Basically you are working with these class members inside the thread functions _temporize and _sleepThenTimeout.

timeout, _isSingleShort and _interval, these three cannot be changed after the start function is called, so it is safe to use them.
_running on the other hand can be read/write by both threads. In reality it might not cause any problem as assignment to bool is atomic (on most architectures), but to be 100% safe you can use std::atomic<bool>.

Another important thing to keep in mind is that while timer class itself is thread safe, it is responsibility of the user of this class to make sure that the timer call back function (_timeout) is thread-safe by it self, as it will be executed in a separate thread.


User function construct user.class I'm trying to create my user.class.php constructor but I don't know really much about POO so I'm going to tell you what I'm doing and I hope you could help me.

user.class.php

class user {
    private $fullname;
    private $age;
    private $email;
    private $sex;
    private $nacionality;

        public function __construct($usrId){
            $mysqli = new connection_mysqli(); // I've already implemented this and it works.
            $query = "SELECT * FROM USERS WHERE USER_ID = $userId";
            if ($result = $mysqli->query($query)) {
               $userData = $result->fetch_assoc();
               $this->fullname = $userData['fullname'];
               $this->age = $userData['age'];
               $this->email = $userData['email'];
               //.....
            }
        parent::__construct($usrId);

    }
}

What I'm going to do on my index.php is this:

index.php

<?php
include('scripts/class/user.class.php');
$user = new user($_SESSION['userid'];
echo $user->fullname;
?>

So, is this going to work right? Is this well-programmed? Any tips?

First of all I recommend you to setup a local webserver to test your php code, if you have not done so jet (I assume you haven't tried your code, since your asking if it's going to work).

Secondly you should abandon mysqli for PDO. Read PHP Database Access: Are You Doing It Correctly?

Also, why are you calling the parent's constructor when the user class has no parent (eg. the class does not extend another class)?


Filter out string sections I have a helper method to clean up a string and remove pieces that I will be generating again. (defined by the "SpecialKey" Constants) but I've never been real happy with the code. Although I find it easy to read adding more keys doesn't always seem so clean.

    protected static System.String[] GetStringParts(System.String stringValue)
    {
        System.String[] stringParts = stringValue.Split('!');

        System.Func<System.String, System.Boolean> isConstKey = (System.String key) =>
        {
            return !System.String.IsNullOrWhiteSpace(key) && (
                    key.StartsWith(MyUtility.SpecialStringKey1) ||
                    key.StartsWith(MyUtility.SpecialStringKey2) ||
                    key.StartsWith(MyUtility.SpecialStringKey3) ||
                    key.StartsWith(MyUtility.SpecialStringKey4)
                );
        };
        return stringParts.Where(key => !isConstKey(key)).ToArray();
    }

I have removed specific names and such. I don't usually go by the convention SpecialKey1/2 etc

I'd like any pointers on performance or readability. This code gets hit often but in the last two years has only been changed once (maybe twice).

I have a couple of suggestions and observations I'd like to point out:

Fully qualified System types reduce readability without adding information.
So: use string[] instead of System.String[], bool instead of System.Boolean and so on.

Defining a lambda statement and storing it in a variable overcomplicates the code. Use query expression syntax instead.

Are you actually using the return value as a string[] (accessing by index, etc.) or could you get away with returning IEnumerable<string>? (Eric Lippert explains why you should.)

Your isConstKey looks wrong, but it's hard to tell because of the double negative. Are you trying to remove empty strings? Currently, you're leaving the white space in the result, not removing it. In below code sample, I've corrected the problem.

Instead of comparing all the SpecialKeyStrings seperately, put them into a sequence exposed as a property of MyUtility and use LINQ to validate against that:
  public static IEnumerable<string> SpecialStringKeys
  {
      get
      {
          return new[] { SpecialStringKey1, SpecialStringKey2, SpecialStringKey3,
                         SpecialStringKey4, };
      }
  }

Edit: other possibilities would be to avoid creating a new array every time; you could create a private static readonly ReadOnlyCollection<string> field or just a private static readonly string[].
All that would allow your GetStringParts method to be changed to the following:
protected static IEnumerable<string> GetStringParts(string stringValue)
{
    return from part in stringValue.Split('!')
           where !string.IsNullOrWhiteSpace(part) &&
                 !MyUtility.SpecialStringKeys.Any(part.StartsWith)
           select part;
}

(If you really need an array, add the ToArray call and change the return type back).
If you want to go one step further in readability, you could extract the condition to an extension method:
public static bool IsNeitherKeyNorWhiteSpace(this string source)
{
    return !string.IsNullOrWhiteSpace(source) &&
           !MyUtility.SpecialStringKeys.Any(source.StartsWith);
}

Changing your original method thus:
protected static IEnumerable<string> GetStringParts(string stringValue)
{
    return from part in stringValue.Split('!')
           where part.IsNeitherKeyNorWhiteSpace()
           select part;
}


Credit card validation with reversed array I want a review of this code.

<?php
  $array = $_POST['array'];  //array holding numbers
  $array = array_reverse($array);          // reversing the array
  echo "
 Credit card Before :- ";
  for($i=0;$i<sizeof($array);$i++)
    echo $array[$i];
  echo "
";
  echo "
 Credit card after applaying mod 10 Algorthim :- ";
  //start converting the number
  for($i=0;$i<sizeof($array);$i++){
    if($i%2==1){
      $array[$i] += $array[$i];
        if($array[$i]> 9 ){

          $array[$i] -= 9;
        }
      }
      //suming all the resulted number
      $total +=$array[$i];
      if($i%2==1)
        echo "".$array[$i]."";
      else
        echo $array[$i];
    }
    echo "
";
      if($total % 10 ==0){
        echo "This creidet card is valid";
      }else{
        echo "This creidet card is not valid";
      }
?>

There's a few glaring issues with your code. Namely your lack of consistent styling. Secondly, you're not doing certain things correctly (iterating over an array rather than imploding it). As this is a code review, I will not be touching on the correctness of the algorithm. Below you'll find two pieces of code - one which I've commented and corrected, and a second which I've removed my comments and old code.

With comments:

<?php

# Grab the numbers array and reverse it.
# Don't just name your variable $array. Name it something meaningful.
$numbers_array = array_reverse( $_POST['array'] );

echo "
 Credit card Before :- ";

# No need to do a for loop, just use implode to make the array into a string
/*for ( $i = 0; $i < sizeof( $array ); $i++ ){
    echo $array[$i];
}*/
echo implode("", $numbers_array);

# Just do one echo instead of two
/*echo "
";*/
echo "

 Credit card after applaying mod 10 Algorthim :- ";

# Start converting the number
# You're doing a sizeof() on every single loop. Just cache it in a $size variable
$size = count($numbers_array);
for ( $i = 0; $i < $size; $i++ ) {
    if ( $i % 2 == 1 ) {
        $numbers_array[$i] += $numbers_array[$i];
        if ( $numbers_array[$i] > 9 ) {
            $numbers_array[$i] -= 9;
        }

        echo "" . $numbers_array[$i] . "";
    } else {
        echo $numbers_array[$i]
    }

    $total += $numbers_array[$i];

    # You're doing the same calculation twice (as above). Why not just echo it up there?
    /*if ( $i % 2 == 1 ){
        echo "" . $numbers_array[$i] . "";
    } else {
        echo $numbers_array[$i];
    }*/
}

echo "
";
if ( $total % 10 == 0 ) {
    echo "This credit card is valid";
} else {
    echo "This credit card is not valid";
}

With no comments:

<?php

# Grab the numbers array and reverse it.
$numbers_array = array_reverse( $_POST['array'] );

echo "
 Credit card Before :- " . implode("", $numbers_array) .
     "

 Credit card after applaying mod 10 Algorthim :- ";

# Start converting the number
$size = count($numbers_array);
for ( $i = 0; $i < $size; $i++ ) {
    if ( $i % 2 == 1 ) {
        $numbers_array[$i] += $numbers_array[$i];
        if ( $numbers_array[$i] > 9 ) {
            $numbers_array[$i] -= 9;
        }

        echo "" . $numbers_array[$i] . "";
    } else {
        echo $numbers_array[$i]
    }

    $total += $numbers_array[$i];
}

if ( $total % 10 == 0 ) {
    echo "
This credit card is valid";
} else {
    echo "
This credit card is not valid";
}


Custom Library (pseudo-jQuery) Well, I'm not going to use it everywhere, as jQuery exists, but still was interested in experience of creating own library. Here are first steps:

;(function() {
  window["_o"] = function(selector) {
    if (!selector) {
      throw new Error("Cannot create Object from " + selector);
    }

    var elems;

    if (selector instanceof HTMLElement) {
      elems = {
        "0": selector,
        length: 1
      };
    } else {
      elems = document.querySelectorAll(selector);
    }

    return elems.length ? new _o.fn(elems) : null;
  }

  _o.fn = function(elems) {
    var len = this.length = elems.length;
    for (var i = 0; i < len; i++) {
      this[i] = elems[i];
    }

    /***/
    /* general */

    this.eq = function(num) {
      var tmp = num;
      num = parseInt(num);
      if (isNaN(num))
        throw new Error(tmp + " is not a number!");

      return 0 <= num && num < elems.length ? _o(elems[num]) : null;
    };

    this.first = function(selector) {
      if (typeof selector === "string") {
        var elem = elems[0].querySelector(selector);
        return elem ? _o(elem) : null;
      }

      return _o(elems[0]);
    };

    /***/
    /* className features */

    this.addClass = function(className, f_condition) {
      f_condition = f_condition || function() {
        return true;
      };

      for (var i = 0; i < elems.length; i++) {
        if (f_condition(elems[i], i, elems)) {
          elems[i].classList.add(className);
        }
      }

      return this;
    };

    this.removeClass = function(className, f_condition) {
      f_condition = f_condition || function() {
        return true;
      };

      for (var i = 0; i < elems.length; i++) {
        if (f_condition(elems[i], i, elems)) {
          elems[i].classList.remove(className);
        }
      }

      return this;
    };

    this.toggleClass = function(className, f_condition) {
      f_condition = f_condition || function() {
        return true;
      };

      for (var i = 0; i < elems.length; i++) {
        if (f_condition(elems[i], i, elems)) {
          elems[i].classList.toggle(className);
        }
      }

      return this;
    };

    /***/
    /* show - hide */

    this.display = function(value, f_condition) {
      if (typeof value === "function") {
        f_condition = value;
        value = "block";
      } else {
        value = value || "block";
        f_condition = f_condition || function() {
          return true;
        };
      }

      for (var i = 0; i < elems.length; i++) {
        if (f_condition(elems[i], i, elems)) {
          elems[i].style.display = value;
        }
      }

      return this;
    };
  }
})();
.demo {
  width: 50px;
  height: 50px;
  margin: 5px;
  background: orange;
}

.blue {
  background: #045acf;
}

Any suggestions / hints?) Also, is it a good or bad practice to write a separate mini-library before the main code?

You have a fn which creates the internal object. Maybe call it something more precise, like Collection or MakeCollection. Since the argument to the fn will be an array or an NodeList, you can concisely assign each property of the argument to the instance with Object.assign, no need for manual iteration.

When possible, consider using dot notation instead of bracket notation - dot notation is shorter, easier to read, and works better with minifiers. See the eslint rule.

Right now, you're creating new methods for the instance every time the library is called. It would be more efficient to put them on the prototype instead, eg:

_o.fn.prototype.eq = function(num) {
  // ...

But since it's 2020, it would make sense to at least write with modern ES2015 syntax, and use a class:

class Collection {
  constructor() {
    // ...
  }
  eq(num) {
    // ...
  }

Using modern syntax often makes code more concise and easier to read, which means less surface area for bugs, as well as more methods available for use. If you need compatibility for obsolete 6+ year old browsers, use Babel to transpile it into ES5 syntax automatically for production (but keep your source code modern!).

In your eq method, something else that keeps code easy to read is when variables aren't reassigned unnecessarily. For example, if you see a variable created at line 5, and you see it used on line 25, if you aren't sure if you're reassigned it, you'll have to look closely to each line 6-24 to be absolutely certain of its value at 25. But even with that, I'm quite doubtful that the numeric verification in the eq provides any benefits - not even jQuery does such a thing. I think you may as well just return the num property on the instance. Worst-case, if the user provides an invalid argument, they'll get undefined in return, which is probably fine as-is.

eq(num) {
  return this[num];
}

Though, if you wanted to emulate jQuery, eq should return a new collection, including the wrapper, not the underlying element:

eq(num) {
  return new Collection(this[num]);
}

If you want the library to be similar to jQuery, something that would be helpful would be if it doesn't throw when the collection is empty. If the elms turn out to be empty, currently, you return null, which means that to be safe, a consumer of the library would often enough have to do something like:

const $elms = $('.foo');
if ($elms) {
  $elms.display('inline-block');
}

instead of unconditionally calling $('.foo').display('inline-block'), which looks much cleaner. The first method has the same issue, though if this is deliberate, that's fine.

Your first method is doing something markedly different from jQuery's - yours takes a selector and searches for a descendant matching the selector, while jQuery's doesn't (jQuery's just extracts the first element in the collection). For those already experienced with jQuery, this has good potential to cause confusion. To find a selector-matching descendant of the first element in the collection, consider renaming your method, maybe to something like firstDescendant.

In each of your class methods, rather than assigning to f_condition if it doesn't exist on the first line:

addClass(className, f_condition) {
  f_condition = f_condition || function() {
    return true;
  };

Consider using default arguments instead:

addClass(className, f_condition = () => true) {

An option to make your addClass, removeClass, toggleClass, and display loops less repetitive would be to have a helper function which iterates over all elements in the collection, and executes a callback if a test provided by a different callback is passed:

_iterate(test, callback) {
  for (let i = 0; i < this.length; i++) {
    if (test(this[i], i, this)) {
      callback(this[i]);
    }
  }
}
addClass(className, f_condition = () => true) {
  this._iterate(f_condition, elm => elm.classList.add(className));
  return this;
}

For the display method, argument overloading is a bit messy in Javascript. You can do it if you want, but I think the code's logic would be easier to follow and it would make more intuitive sense to consumers if an object with multiple optional properties was passed instead:

display({ newDisplay = 'block', test = () => true } = {}) {
  this._iterate(test, elm => elm.style.display = newDisplay)
  return this;
}

Another benefit to this is that you don't need to reassign anything.

Put it all together:

;(function() {
  window._o = function(selector) {
    if (!selector) {
      throw new Error("Cannot create Object from " + selector);
    }
    return new _o.Collection(
      selector instanceof HTMLElement
        ? [selector]
        : document.querySelectorAll(selector)
    );
  }
  _o.Collection = class Collection {
    constructor(elems) {
      Object.assign(this, elems);
      this.length = elems?.length || 0;
    }
    *[Symbol.iterator]() {
      for (let i = 0; i < this.length; i++) {
        yield this[i];
      }
    }
    eq(num) {
      return new Collection(this[num]);
    }
    first() {
      return new Collection(this[0]);
    }
    firstDescendant(selector) {
      // don't forget to check if elms[0] exists first:
      return new Collection(this[0]?.querySelector(selector));
    }

    _iterate(test, callback) {
      for (let i = 0; i < this.length; i++) {
        if (test(this[i], i, this)) {
          callback(this[i]);
        }
      }
    }
    /***/
    /* className features */
    addClass(className, f_condition = () => true) {
      this._iterate(f_condition, elm => elm.classList.add(className));
      return this;
    }
    removeClass(className, f_condition = () => true) {
      this._iterate(f_condition, elm => elm.classList.remove(className));
      return this;
    }
    toggleClass(className, f_condition = () => true) {
      this._iterate(f_condition, elm => elm.classList.toggle(className));
      return this;
    }

    /***/
    /* show - hide */
    display({ newDisplay = 'block', test = () => true } = {}) {
      this._iterate(test, elm => elm.style.display = newDisplay)
      return this;
    }
  };
})();
.demo {
  width: 50px;
  height: 50px;
  margin: 5px;
  background: orange;
}

.blue {
  background: #045acf;
}

  write a separate mini-library before the main code?

You can if you want a bit more experience figuring out how such a library would be made, but for anything you expect to maintain (or that others will maintain), I'd recommend using one of the common libraries that are already tried-and-tested and have thorough documentation, Q+A, and support (like jQuery). Otherwise, any reader of the code will first have to internalize your library before actually getting to any of the meat of what the main script does. It's a good amount of overhead without any real benefit. Not that you can't create helper functions (or an object of helper functions) for yourself, but it's not the best idea to reinvent the wheel with serious code.


Improving performance when sorting array of structs by multiple fields I have an array of a struct containing 5 byte values.
I need to sort this array by each value in a different order depending on the situation.
These arrays are sometimes quite small (10-30 elements), sometimes quite large (1024-16384).

Below is the code I'm using. The reason this has become a bottleneck for me is due to having to make it an array again which reallocates memory for it if I understand correctly (for both the sort and turning it back to an array). My desire is to sort the array the way I want (which can be one of the 3 below situations) without having to do something like ToArray and instead populate the array that exists.

I may be entirely wrong about sorting like this or my analysis of the problem, which is why I'm here. If anyone can point me to a better way to do this, please let me know. This program needs to be able to call this routine hundreds (sometimes thousands) of times per second.

public enum eCubeType : byte {
    Air = 0,
    Dirt = 1
}

public struct strCubeFace {
    public byte X, Y, Z, LightLevel;
    public eCubeType CubeType;
}

Function call snippet including array sorting code:

        if (iSide == 0 || iSide == 1) {
            Faces = Faces.OrderBy(c => c.Y).ThenBy(c => c.CubeType).ThenBy(c => c.LightLevel).ThenBy(c => c.X).ThenBy(c => c.Z).ToArray();
            SmartMesh0();
        }
        if (iSide == 2 || iSide == 3) {
            Faces = Faces.OrderBy(c => c.Z).ThenBy(c => c.CubeType).ThenBy(c => c.LightLevel).ThenBy(c => c.X).ThenBy(c => c.Y).ToArray();
            //SmartMesh1();
        }
        if (iSide == 4 || iSide == 5) {
            Faces = Faces.OrderBy(c => c.X).ThenBy(c => c.CubeType).ThenBy(c => c.LightLevel).ThenBy(c => c.Y).ThenBy(c => c.Z).ToArray();
            //SmartMesh2();
        }

Edit for @svick or anyone else wanting to know more details:

This is in regards to a 3D procedural map generator that expands as the player moves outwards. The terrain is geometrically similar to that of Infiniminer or Minecraft. This lends itself well to combining like faces of the cube based terrain to lower vertices by 50% or higher (usually higher).

After generating the points I need to represent each cube, it's light level, and so on; I then iterate over every solid block that is adjacent to transparent/translucent blocks to create the strCubeFace structure I use to represent individual faces.

Unless I am simply doing this whole process in a very "bad" way for performance, I need to sort 6 of these arrays per chunk (grouping of 32x256x32 cubes), one for each face. It is indeed always a different array each time it is needed to be sorted. It doesn't necessarily need to be an array, but as I already know the total number of cubes adjacent to transparent/translucent blocks, setting up a simple array of the max size made the most sense to me (once the array is populated, if the number of faces in each specific array is larger than the array's length, I simply re-size the array down prior to the sort). It is my belief that most other collection storage techniques would incur a higher processing penalty or a higher memory storage penalty.

The only reason I am sorting these to begin with, which is probably most important to know/understand, is so that I can iterate over the sorted array of faces to very quickly/easily create the vertices necessary to combine like faces.

Sorry for rambling so much, but this is a rather big project and I'm not so good at condensing down my reasoning for any of these things without nearly writing a paper on the subject since it's all so intertwined. I'd nearly just post the whole project for review, but that would seem inappropriate (that and my code is borderline illegible at times as I'm currently just a hobbyist).

If your analysis is correct and the slowdown is really cased by the copying, then you could avoid that by using Array.Sort(), which directly sorts the array you have.

Though Array.Sort() isn't as convenient as OrderBy() combined with ThenBy(), you will have to create an IComparer or Comparison for each of the possible sort orders. It could look something like this:

Comparison<eCubeType> yFirstComparison = (c1, c2) =>
{
    int result = c1.Y.CompareTo(c2.Y);
    if (result != 0)
        return result;

    // ...

    return c1.Z.CompareTo(c2.Z);
}

If you don't like to do this manually you can use NList (the project site seems to be down at the moment):

IComparer<eCubeType> yFirstComparer = KeyComparer<eCubeType>.OrderBy(c => c.Y)
    .ThenBy(c => c.CubeType).ThenBy(c => c.LightLevel).ThenBy(c => c.X).ThenBy(c => c.Z);


Pizza Hut math problem #1 in Haskell I'm currently attempting to learn Haskell after a fair history of imperative programming. When I saw the Pizza Hut Pi Day math problems, I thought the first one would be a good candidate for my Haskell experimenting.  The question is:

  I’m thinking of a ten-digit integer whose digits are all distinct. It happens that the number formed by the first n of them is divisible by n for each n from 1 to 10. What is my number?

And I came up with the following code:

module Main where
import System.Environment

import Data.List

pizza2 = [ x | x <- [1000000000..9999999999],
            (x `div` (10 ^ 0)) `mod` (10) == 0,
            (x `div` (10 ^ 1)) `mod` (9) == 0,
            (x `div` (10 ^ 2)) `mod` (8) == 0,
            (x `div` (10 ^ 3)) `mod` (7) == 0,
            (x `div` (10 ^ 4)) `mod` (6) == 0,
            (x `div` (10 ^ 5)) `mod` (5) == 0,
            (x `div` (10 ^ 6)) `mod` (4) == 0,
            (x `div` (10 ^ 7)) `mod` (3) == 0,
            (x `div` (10 ^ 8)) `mod` (2) == 0,
            (x `div` (10 ^ 9)) `mod` (1) == 0,
            nub ( show x ) == show x]

main :: IO ()
main = do
putStrLn (show (nub (pizza2)))

To my suprise, this actually compiles, runs, and returns the correct result. However, due to my inexperience (I think), it takes about 9 minutes to run and I feel like it should take less than a minute on the long side and just a couple seconds on the short side.

Two questions:

Why does this take so long?
I really feel like all those extra conditions I wrote should be able to be abstracted into one condition, but I couldn't figure it out on my own. Any ideas on how to condense that into one, more abstract condition?

Basically, you're testing a lot of paths you don't have to.  It would probably be more efficient to find a 1-digit number that is divisible by 1, then use that digit to find a 2-digit number that is divisible by 2, then use those two digits to find a 3-digit number that is divisible by 3, then recursively backtrack if you run out of options at one level.

Here's some code to illustrate.  It returns essentially instantly in my REPL.  You call it by pizza 1 0 initially.

import Data.List ((\\))
import Data.Char (digitToInt, intToDigit)
import Data.Maybe (listToMaybe, mapMaybe)

pizza :: Integer -> Integer -> Maybe Integer

pizza 10 accum = Just (accum * 10)

pizza n accum =
  let usedDigits = map digitToInt $ show accum
      unusedDigits = map toInteger $ [1..9] \\ usedDigits
      previous = accum * 10
      divisibleDigits = filter (\digit -> (previous + digit) `mod` n == 0) unusedDigits
  in listToMaybe $ mapMaybe (\digit -> pizza (n + 1) (previous + digit)) divisibleDigits

However, for one-shot problems like these, your solution is fine.  If you can't write a faster algorithm in less than 9 minutes (of your programmer time), you haven't actually saved any time.


Check directed graph is tree or not Here is my approach.
1. Checking there is no cycle.(Using BFS)
2. All nodes are connected. (visited)

public static boolean isTree(int[][] adjMat) {
    int numVertex = adjMat.length;
    State[] isVisited = new State[numVertex];
    for (int i = 0; i < numVertex; i++) {
        isVisited[i] = State.UNVISITED;
    }

    // use BFS
    Queue q = new LinkedList<>();
    q.add(0);
    isVisited[0] = State.VISITING;
    while (!q.isEmpty()) {
        int current = (int) q.poll();
        if (isVisited[current] == State.VISITED) {
            return false;
        }
        isVisited[current] = State.VISITING;
        for (int i = 0; i < numVertex; i++) {
            if (adjMat[current][i] == 1) {
                q.add(i);
            }
        }
        isVisited[current] = State.VISITED;

    }
    for (int i = 0; i < numVertex; i++) {
        if (isVisited[i] == State.UNVISITED)
            return false;
    }
    return true;

}

public enum State {
    UNVISITED, VISITING, VISITED
}

Any improvements/suggestions please.

Task solution

Whole algorithm you created assumes node with label 0 is a root. It will give you false negative on other instances. This may be part of the assignment though, so will not be judge of that.

In case it is not, use method of gradual removal of leaves. First detect all leaves putting them in process queue. Then gradually remove those leaves, and if a neighbour of removed leaf becomes a leaf itself, add it to queue in the next round. If given graph is a tree, you should end up either with empty set of nodes, or root of the tree.

Data structures

In case it is sufficient, use byte instead of int arrays. As opposed to byte variable (1 cell = 4 bytes), byte array is actually spread to have 1 cell per 1 byte of memory. This will crank up your cache line hit chances, as more of the used information is in the cache at the same time. This pays both for input array (which you may not be able to change) but also for visited array.

Enum is all fine and dandy, but there is no good reason for enum that is used solely inside of a relatively small function. Especially when name of said enum is so generic as "state". At least make it private is you want to use it.

BFS vs DFS

BFS and DFS have the same complexity, and in this case, they would yield the same result. The difference is in performance though. In case of simple linked list queue, you create a lot of objects for starters, which is quite expensive operation in itself, that are all over the place in terms of memory, so two items in the queue will be rarely on the same cache line, and thus no hits.

For BFS, you could implement array rotational queue (limited size), where on poll you increment head and on add increment tail (both in modulus size Q, which should be power of 2 to have boost from masking instead of modulus itself). This will give you some hits from both sides of the queue.

As I mentioned, using DFS is also an option. But somewhat better, since you have array stack, fixed size again, but no modulus, shifting only one pointer. This will ensure that you operate only on one cache line, as opposed to two of them in case of queue.

The size in both cases does not need to be larger than N, number of nodes in the graph.

Pruning

When you expand, you always add a node in the queue, even though it may already be there. The thing is, if a node is inside of the queue, you already know you will encounter it twice, once marking it visited, second time finding out it is visited and terminating with false.

You can thus check whether an item is in queue (different flag) and save yourself all the computation that might come in between.

Next, the loop at the end where you are checking whether all nodes were visited at least once, it is connectivity checking. You can do that much faster, if you will introduce a counter, that is incremented (use pre-increment) every time you mark the node as visited. Then if you are checking at the end overall connectivity, you can just compare this counter to number of nodes, and if they do not match, a node was omitted. This way, you'll save N conditionals (that are well predicted, but still worse than addition), and pay for it with addition operation, which is not tied to anything else in the position of use, so it is well pipelined.

First loop can be removed as well, since removal of enum, and stating that 0 is unvisited, simple allocation initialized the array to correct values.

Conclusion

It is hard to optimize this, since complexity of DFS/BFS on adjacency matrix graph is O(N^2), so everything is kind of shaded by that.

I hope this "all over the place" text helped at least a bit.


How do I refactor lines of Ruby code that run too long due to method chaining or object instantiation? Here are a couple of examples of one-liners that go way beyond 80 characters:

scope = DepartmentRepository.includes(:location).by_account(@request.account_id).find(approved_department_ids)
departments = ActiveModel::ArraySerializer.new(scope, each_serializer: DepartmentWithLocationSerializer).as_json

I usually simply break these kinds of chained method calls before the . and indent the next line. My IDE (Emacs) runs rubocop on my source files as I edit them and doesn't complain about this practice. It does complain about lines longer than 80 characters so I feel somewhat compelled to fix them. How would I refactor these? The first line is calling ActiveRecord finders and scopes. Scopes can be compbined but then they have ackward names that seldom see reuse (e.g. by_account_with_locations).

I don't really see any real trouble with those lines. But I know the feeling that a line just seems "too long" (especially if everything else in the file is nice and neat). It's often a good thing to notice, but in some cases it just doesn't make sense to worry.

Are the lines readable? Absolutely. They are very descriptive in fact. If you were talking about a very long conditional filled with && and || and negations, then yeah, it should probably be changed. But in this case, I don't see reason to fret.

But if you're intent on refactoring, you could (as an example) move the ArraySerializer instantiation into a factory on DepartmentWithLocationSerializer and get something like

DepartmentWithLocationSerializer.serialize(scope).as_json

That sort of thing helps a bit. But if I knew a good zen quote about a little imperfection being OK too, I'd drop it here :)

Anyway, don't worry about adding methods for things you'll only use once. You don't always need a very "practical" reason like DRY, to add some more methods. If adding some methods it can make the code a little neater to look at, that's a reason in itself. Everything in moderation, YMMV, etc., so just try it out and see if the code seems nicer afterwards. If it doesn't, well, then try to live with the long lines.


MCU (Embedded C): Use nested if/elses or use an ugly function? I'm writing driver code for the SPI peripheral (as part of a tutorial) on a Nucleo64 (STM32F446RE). The instructor recommends writing this code:

// General macros
#define EnableStatus uint8_t
#define DISABLE      0
#define ENABLE       !DISABLE

void SPI_EnablePeripheralClock(SPI_RegDef_t *pSPIx, EnableStatus status)
{
    if (status) {
        if (pSPIx == SPI1)
            SPI1_PCLK_ENA();
        else if (pSPI == SPI2)
            SPI2_PCLK_ENA();
        // ... done for 4 SPI modules
    } else {
        if (pSPIx == SPI1)
            SPI1_PCLK_DIS();
        else if (pSPI == SPI2)
            SPI2_PCLK_DIS();
        // ... done for 4 SPI modules
}

...
// You can treat RCC->APB2ENR as the value at some address. ENA() sets a bit. DIS() clears a bit.
#define SPI1_CLK_ENA()       (RCC->APB2ENR |= (1 << 12))
#define SPI2_CLK_ENA()       (RCC->APB1ENR |= (1 << 14))
#define SPI1_CLK_DIS()       (RCC->APB2ENR &= ~(1 << 12))
#define SPI2_CLK_DIS()       (RCC->APB1ENR &= ~(1 << 14))
...
#define IO volatile

typedef struct {
    IO uint32_t CR1;        // Control Reg. 1
    IO uint32_t CR2;        // Control Reg. 2
    IO uint32_t SR;         // Status Reg.
    IO uint32_t DR;         // Data Reg.
    IO uint32_t CRCPR;      // CRC Polynomial Reg.
    IO uint32_t RXCRCR;     // RX CRC Reg.
    IO uint32_t TXCRCR;     // TX CRC Reg.
    IO uint32_t I2SCFGR;    // I2S Configuration Reg.
    IO uint32_t I2SPR;      // I2S Prescaler Reg.
} SPI_RegDef_t;
#define SPI1 ((SPI_RegDef_t*)SPI1_BASEADDR)
#define SPI2 ((SPI_RegDef_t*)SPI2_BASEADDR)

Several things I don't like about this implementation:

The chain of if-elses in the function to set or clearing one bit. It's a lot of lines-of-code to express a simple task.
It's comparing peripheral device base addresses to figure out which of the four SPI peripheral it's working with.

I rewrote it like this to address the issues:

// Assigns the boolean value b to the p'th bit position (0-based indexing) of the number `num`.
void BitAssign(volatile uint32_t *num, uint8_t p, bool b)
{
    // https://stackoverflow.com/a/47990/3396951
    b = !!b; // booleanize b (if it was previously non-zero, it is now 1 otherwise it's 0)
    *num = (*num & ~(1UL << p)) | (b << p);
}

// To be used as first argument in SPI_EnablePeripheralClock() and other functions
#define SPI1 1
#define SPI2 2
#define SPI3 3
#define SPI4 4

void SPI_EnablePeripheralClock(uint8_t SPIx, EnableStatus status)
{
    // 12, 14, 15, 13 are values provided by the datasheet
    if (SPIx == SPI1)
        BitAssign(&RCC->APB2ENR, 12, status);
    else if (SPIx == SPI2)
        BitAssign(&RCC->APB1ENR, 14, status);
    else if (SPIx == SPI3)
        BitAssign(&RCC->APB1ENR, 15, status);
    else if (SPIx == SPI4)
        BitAssign(&RCC->APB2ENR, 13, status);
}

This avoids relying on the ENA()/DIS() functions (I can remove them) and utilizes a bit-operation to perform the bit assignment. I'm unsure if  BitAssign() is a good idea since I might have to overload it for non-volatile parameters.

The variable names in both of those snippets are absolutely insane! But at least it seems clear on close inspection that ENA and DIS are abbreviations of "enable" and "disable." I would write those out.

I would rewrite the instructor's code like this:

typedef uint8_t EnableStatus;

void SPI_EnablePeripheralClock1(EnableStatus status) {
    if (status) {
        RCC->APB2ENR |= (1 << 12);
    } else {
        RCC->APB2ENR &= ~(1 << 12);
    }
}

void SPI_EnablePeripheralClock2(EnableStatus status) {
    if (status) {
        RCC->APB1ENR |= (1 << 14);
    } else {
        RCC->APB1ENR &= ~(1 << 14);
    }
}

void SPI_EnablePeripheralClock(SPI_RegDef_t *pSPI, EnableStatus status) {
    switch (pSPIx) {
        case SPI1: SPI_EnablePeripheralClock1(status); break;
        case SPI2: SPI_EnablePeripheralClock2(status); break;
        default: assert(false); break;  // what goes here?
    }
}

But it really depends on how this SPI_EnablePeripheralClock function is going to be used "upstream" in the caller. For example, why is EnableStatus a typedef for uint8_t instead of bool?

In fact, I would be tempted to write two convenience functions with English names:

void EnablePeripheralClock() {
    SPI_EnablePeripheralClock(&spi, true);
}
void DisablePeripheralClock() {
    SPI_DisablePeripheralClock(&spi, false);
}

Alternatively, it might make sense to refactor along those lines from the very beginning...

void EnablePeripheralClock() {
    switch (pSPIx) {
        case 1: RCC->APB2ENR |= (1 << 12); break;
        case 2: RCC->APB1ENR |= (1 << 14); break;
        default: assert(false);
    }
}

void DisablePeripheralClock() {
    switch (pSPIx) {
        case 1: RCC->APB2ENR &= ~(1 << 12); break;
        case 2: RCC->APB1ENR &= ~(1 << 14); break;
        default: assert(false);
    }
}

Here we're breaking down the code from the top down, focusing on "what task needs accomplishing" (e.g. "disable the clock"), rather than taxonomizing from the bottom up. Of course it's hard to know if we've done it right, when we don't know what top-level tasks need accomplishing. :)


A Simple Caesar Cipher in Python I wrote an encryption/decryption algorithm for the Caesar Cipher. It works fine, but I'm curious if there are any constructive critiques, tips, tricks, comments, or advice anyone may have on how the code operates, performs, looks, etc.

# Caesar Cipher
def chooseMode():
    while True:
        mode = raw_input("\nChoose a mode (encryption or decryption):\n 1) Encrypt \n 2) Decrypt\n 3) Brute Force\n\n")
        if mode in ['1', '2', '3']:
            return mode

def collectMessage():
    message = raw_input("Enter the message you would like to translate:\n\n")
    return message

def collectKey():
    while True:
        key = raw_input("What is the key for your message? (Enter a number from 0 to 26): \n")
        try:
            if int(key) in range(27):
                return int(key)
            else:
                print("Please enter an integer between 0 and 26.\n")
                continue
        except ValueError:
            print("Please enter an integer between 0 and 26.\n")
            continue

def translateMessage(text, cipher_mode, caeser_key):
    translation = ''
    # If decrypting, convert key to negative.
    if cipher_mode == '2':
        caeser_key = -caeser_key

    for symbol in text:
        # Convert only Alphabetical Symbols
        if symbol.isalpha():
            num = ord(symbol)

            # Shift num down to 1 through 26 for more easier to visualize modulous.
            # Then Apply the Key; then Modulate; then Move Back Up; then Build into Chr and Translation
            if symbol.isupper():
                num -= ord('A')
                num += caeser_key
                num %= 26
                num += ord('A')
                translation += chr(num)

            else:
                num -= ord('a')
                num -= caeser_key
                num %= 26
                num += ord('a')
                translation += chr(num)
        else:
            translation += symbol
    return translation

def bruteForce(text, cipher_mode):
    if cipher_mode == '3':
        for index in range(26):
            print "A Caeser Key of ", str(index), "reveals:\t", translateMessage(text, cipher_mode, 26 - index)
        exit()
    else:
        return None

choice_of_mode = chooseMode()
entered_message = collectMessage()

bruteForce(entered_message, choice_of_mode)

entered_key = collectKey()
final_form = translateMessage(entered_message, choice_of_mode, entered_key)
print final_form

Some style notes (you can read more 'bout them on Python's official style-guide which is called PEP8):

you misspelled Caeser. It should be Caesar.
function names should be snake_cased not camelCased
instead of commenting lines at the middle of the method, just add a docstring:

def choose_mode():
    """
    Docstring here
    """
    # the rest of your code

you should add the if __name__ == '__main__' guard

About the code

It's pretty weird to use exit which is a helper for the interactive shell instead of sys.exit which is intended for use in programs. I'd  stick to the latter.
In collectMessage() you can directly return raw_input("Enter the message you would like to translate:\n\n")
In collectKey(): you can remove both continue statements
You should use format() when printing. Here, you can read more about formatting.

About the algorithm

In Python, we have the translate method which applies a substitution cipher to a string. More, when building the translation table, in Python 2, we have string.maketrans.

import string
from string import ascii_lowercase

shift = 6
cipher = string.maketrans(ascii_lowercase, ascii_lowercase[shift:] + ascii_lowercase[:shift])

print "I love to review code at midnight because I have no life".translate(cipher)

Which outputs:

  I rubk zu xkbokc iujk gz sojtomnz hkigayk I ngbk tu rolk

Another built-in that you may take advantage of is:

"I love to review code at midnight because I have no life".encode("rot13")

PS: You can also have a quick look at this answer for other recommendations that might apply quite well to your code.


JavaScript form validation: improve on 'good, but not good enough' I recently received a 'good, but not good enough' rating on a coding exercise I was asked to perform. It was to validate that if "Product complaint" option was selected that Product name, Product size, Use-by date and Batch code fields all needed to have values. What can I do to improve the JS to go from 'good, but not good enough' to 'great'?

JS:

document.querySelector("form").addEventListener("submit", function(e) {

  var enquiryType = document.querySelector("#enquirytype");

  if (enquiryType.value === "pc") {

      validateProductDetails();
      e.preventDefault();

  }

  });

  function validateProductDetails() {

  var requiredFields = document.querySelectorAll('[id^="product-"]'),
      requiredLabel = document.querySelectorAll('[for^="product-"]'),
      formSubmit = document.forms[0],
      requiredIsEmpty = [],
      i;

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

      if (requiredFields[i].value.length === 0) {

          alert('Please enter the ' + requiredLabel[i].innerHTML + ".");
          requiredFields[i].focus();
          requiredIsEmpty.push(requiredFields[i]);
          return false;

      }

  }

  if (requiredIsEmpty.length === 0) {

      formSubmit.submit();
      return true;
  }

}

HTML:

<form action="/" method="get">

  Enquiry type
  <select name="enquiry" id="enquirytype" required>
    <option value="">Choose</option>
    <option value="ge">General enquiry</option>
    <option value="pf">Product feedback or enquiry</option>
    <option value="pc">Product complaint</option>
  </select>

  Product name
  <input type="text" id="product-name">

  Product size
  <input type="text" id="product-size">

  Use-by date
  <input type="text" id="product-use-by">

  Batch code
  <input type="text" id="product-batch">

  <input type="submit" value="Submit" id="form-submit">

 </form>

Separation of Concerns
So I agree with rolfl, but I also want to expand a bit, and provide some small bit of skeleton code for you to go on.  When I do validation on the client, I usually set up variables for errors, conditions, and dependencies.  This allows me to use a simple handling structure that can be used if needed (as @rolfl suggested), and ignored if not, that way you aren't validating fields that don't need validation, or if they do, you're only performing exactly as much validation as needed.
This simple "boilerplate" that I've just created will also allow you to inject functionality to any of your variables by adding objects to your arrays.  As long as your validate function executes the conditions and dependencies injected, you should have a very nice reusable client validation boilerplate.
Then, you just attach an event listener to whatever form element you want to validate, and run a validate function on that callback.  It'll efficiently go through conditions that exist on elements, instead of the other way around, and you can be as flexible you want with your view code without tying it to your validation function.
One of the key principles of coding is Separation of Concerns.  Any of your code that performs app logic (the validation) shouldn't have any sort of view code in it if at all possible.  I believe this is the biggest single concept that would have made the difference in the feedback you received . If you maintain SOC in this case, no matter how the form changes, your validate function won't have to change, or will be at least very flexible, and if you all of a sudden needed to use the same form but on a mobile page, you could just change the renderErrors function to alter your view code to fit the new display without changing anything else.
That's what I would consider great.  See my fiddle: http://jsfiddle.net/LongLiveCHIEF/sqwYT/
I'll update it a bit, but wanted to give you a head start on my line of thought.


Geometry Approximation of an Integral I am looking for help for two things:
1.how precise this approximation is (such as Big O or percent error)
2. How efficient the algorithm for time / space usage.
I created an algorithm to estimate an integral. The algorithm creates squares and adds the estimate of the remain sum times the area together. Since this function is 2d, the area is multiplied by f(x, y) at a point. The function in this program is f(x, y) = x + (a * y).
The documented code is below.

def f(x:float, y:float, a:float)->float:
    """
    param x:float, x within the function domain
    param y:float, y within the function domain
    param a:float, a paramter of the curve
    Output: Number of function at the point
    """
    return x + (a * y)

class Square:
    def __init__(self, xi:float, yi:float, xf:float, yf:float)->None:
        """
        param xi:float, smaller coordinate of x
        param yi:float, smaller coordinate of y
        param xf:float, larger cooordinate of x
        """
        self.xi = xi
        self.yi = yi
        self.xf = xf
        self.yf = yf

    def estimate(self, a:float)->float:
        """
        param a:float, a from the curve paramter
        Output: a term to add to estimate the intergal
        """
        xd = self.xf - self.xi
        yd = self.yf - self.yi
        if xd < 0:
            xd = xd * -1
        if yd < 0:
            yd = yd * -1
        area_delta = xd * yd
        est = f(self.xi, self.yi, a)
        return area_delta * est

def main()->None:
    """"
    Input: None
    Output:None
    Note: driver program
    """
    x = [1.0, 2.0, 3.0]
    y = [2.0, 3.0, 4.0]
    a = 1.0
    add = 0.0
    square = None
    for row in range(len(x)):
        for col in range(len(x)):
            if row <= col:
                continue
            square = Square(x[row], y[row], x[col], y[col])
            add = add + square.estimate(a)
    print(add)

if __name__ == "__main__":
    main()

``

You are just re-implementing the abs built-in here. (If a number is negative make it positive):
    xd = self.xf - self.xi
    yd = self.yf - self.yi
    if xd < 0:
        xd = xd * -1
    if yd < 0:
        yd = yd * -1

In general I do not understand why you have a whole class defined. I think this should be written more compactly because the actual calculations performed are very simple.
To be more user friendly I would have a function having as input the domain of integration and the function to integrate like:
def weighted_area(xrange, yrange, function, precision):

An argument should be the precision in case the user wants to change the integration step.


Project Euler #4 - Largest Palindrome Product A palindromic number reads the same both ways. The largest palindrome
  made from the product of two 2-digit numbers is 9009 = 91 × 99.

  Find the largest palindrome made from the product of two 3-digit
  numbers.

How would you improve the following code?

I'm looking specifically for:

performance optimizations
ways to shorten the code
more "pythonic" ways to write it

   def is_palindrome(num):
        return str(num) == str(num)[::-1]
    def fn(n):
        max_palindrome = 1
        for x in range(n,1,-1):
            if x * n < max_palindrome:
                break
            for y in range(n,x-1,-1):
                if is_palindrome(x*y) and x*y > max_palindrome:
                    max_palindrome = x*y
                elif x * y < max_palindrome:
                    break
        return max_palindrome

    print fn(999)

The function is_palindrome converts num to a string twice. You might consider:

def is_palindrome(n):
   """Return True if n is a palindrome; False otherwise."""
   s = str(n)
   return s == s[::-1]

And then for the main loop I would write:

from itertools import product
max(x * y for x, y in product(range(1000), repeat=2) if is_palindrome(x * y))

This runs in less than a second so I think it is not worth putting lots of effort into optimizing it.


Converting Morse Code Challenge

Write a program which reads a file containing Morse Code and outputs the conversion.

Specifications

The first argument is a path to a file.
The file contains multiple lines.
Each line is a string of morse code.

Each letter is separated by a space.
Each word is separated by two spaces.

Constraints

The input file is correctly formatted.
The Morse Code strings are alphanumeric.

Sample Input

.- ...- ..--- .-- .... .. . -.-. -..-  ....- .....
-... .... ...--

Sample Output

  AV2WHIECX 45
  BH3

Source

My Solution:

import java.io.File;
import java.io.FileNotFoundException;
import java.util.HashMap;
import java.util.Map;
import java.util.Scanner;

public class Solution {
    private static final Map<String, Character> morseAlphabet = new HashMap<>();

    static {
        morseAlphabet.put(".-", 'A');
        morseAlphabet.put("-...", 'B');
        morseAlphabet.put("-.-.", 'C');
        morseAlphabet.put("-..", 'D');
        morseAlphabet.put(".", 'E');
        morseAlphabet.put("..-.", 'F');
        morseAlphabet.put("--.", 'G');
        morseAlphabet.put("....", 'H');
        morseAlphabet.put("..", 'I');
        morseAlphabet.put(".---", 'J');
        morseAlphabet.put("-.-", 'K');
        morseAlphabet.put(".-..", 'L');
        morseAlphabet.put("--", 'M');
        morseAlphabet.put("-.", 'N');
        morseAlphabet.put("---", 'O');
        morseAlphabet.put(".--.", 'P');
        morseAlphabet.put("--.-", 'Q');
        morseAlphabet.put(".-.", 'R');
        morseAlphabet.put("...", 'S');
        morseAlphabet.put("-", 'T');
        morseAlphabet.put("..-", 'U');
        morseAlphabet.put("...-", 'V');
        morseAlphabet.put(".--", 'W');
        morseAlphabet.put("-..-", 'X');
        morseAlphabet.put("-.--", 'Y');
        morseAlphabet.put("--..", 'Z');
        morseAlphabet.put("-----", '0');
        morseAlphabet.put(".----", '1');
        morseAlphabet.put("..---", '2');
        morseAlphabet.put("...--", '3');
        morseAlphabet.put("....-", '4');
        morseAlphabet.put(".....", '5');
        morseAlphabet.put("-....", '6');
        morseAlphabet.put("--...", '7');
        morseAlphabet.put("---..", '8');
        morseAlphabet.put("----.", '9');
    }

    public static void main(String[] args) {
        if (args.length != 1) {
            System.err.println(args.length > 1 ? "Excessive arguments, only the first will be considered" : "No arguments provided.");
            System.exit(1);
        }

        try (Scanner fileScanner = new Scanner(new File(args[0]))) {
            while (fileScanner.hasNextLine()) {
                System.out.println(decode(fileScanner.nextLine()));
            }
        } catch (FileNotFoundException fnfe) {
            fnfe.printStackTrace();
        }
    }

    private static String decode(String morse) {
        StringBuilder decoded = new StringBuilder();

        String[] words = morse.split("\\s{2}");
        for (String word : words) {
            decoded.append(' ');
            String[] letters = word.split("\\s");
            for (String letter : letters) {
                decoded.append(morseAlphabet.get(letter));
            }
        }

        return decoded.substring(1);
    }
}

Tests:

import org.junit.Test;
import static org.junit.Assert.assertEquals;

public class TestMorseConversion {
    @Test
    public void testCodeReviewMorse() {
        assertEquals(Morse.decode("-.-. --- -.. .  .-. . ...- .. . .--"), "CODE REVIEW");
    }

    @Test
    public void test2ndMonitor() {
        assertEquals(Morse.decode("..--- -. -..  -- --- -. .. - --- .-."), "2ND MONITOR");
    }

    @Test
    public void testFromCodeEval() {
        assertEquals(Morse.decode("..-. .-. --- --  -.-. --- -.. . . ...- .- .-.."), "FROM CODEEVAL");
    }
}

Note: The Morse class simply contains the map and decode method utilized above to facilitate testing.

Debatable ... and a bug?

constructs like these are one of those things that personal-preference plays in to, but consider:

    if (args.length != 1) {
        System.err.println(args.length > 1 ? "Excessive arguments, only the first will be considered" : "No arguments provided.");
        System.exit(1);
    }

Error handling is boiler-plate that can be made more readable by separating the error conditions out explicitly. In checking/refactoring this, I found an apparent bug, too (which was hard to spot because of the compound exit condition). The bug is that if the arg-count is more than 1, you print the message "Excessive arguments, only the first will be considered" but then terminate anyway..... so much for ignoring the subsequent args...  I would prefer:

private static final void terminate(String message) {
    System.err.println(message);
    System.exit(1);
}

.....

    if (args.length == 0) {
        terminate("No arguments provided");
    }
    if (args.length > 1) {
        terminate("Too many arguments provided. Expect exactly 1");
    }

A distinct advantage I find when you separate out the error conditions, is that maintaining them is simpler, especially when using version-control because the diffs are simpler to understand.

Challenge

Programming challenges are often time-constrained. While for most purposes your code is fast enough, there's a simple technique that can significantly improve performance - do bulk operations (at the expense of using memory). Specifically, it's typically better to "accumulate" the output in a "StringBuilder" and then have a single System.out.println() at the end. The in-the-loop println you currently have will cause the system to "flush" each line, and often that amounts to significant portions of your program time (more than half?).

Similarly, reading in the entire input file in one "gulp" is often faster than a Scanner, or some other operation.

Obviously, "huge" inputs may be a problem, in theory, but I have never yet been found victim to conditions like that in challenges.

Gulping

I would consider using a gulp function to read the entire file - the Java 8  java.nio.files package can help here, and I would choose:

List<String> data = Files.readAllLines(Paths.get(args[0]));

Splitting

A trick of loading an empty-string in to your decode-routine to output a space, will allow you to use a regex to split, and process each line, in a stream:

private static final Pattern charsplit = Pattern.compile(" ");

and then:

String out = charsplit.splitAsStream(line)
             .map(letter -> morseAlphabet.get(letter))
             .collect(Collectors.joining(""));

You can then append that out with a newline, to the output buffer... so that the whole thing ends up looking a little like (untested - see conclusion for working code):

String output = Files.readAllLines(Paths.get(args[0])).stream()
     .map(line -> charsplit.splitAsStream(line)
                           .map(letter -> morseAlphabet.get(letter)
                           .collect(Collectors.joining("")))
     .collect(Collectors.joining("\n");

System.out.println(output);

I would probably take that inner stream and make a real Function<String,String> out of it. I would also make the printing a separate function ... it should not be part of the parsing/decoding.

Conclusion

I put together a working example program here: https://ideone.com/hCpUA1

Note that the version of the code I was happiest with separated out the various functions, and converted the Map<String, Character> to a Map<String, String>. Note that the map also has the "Space" value in it.

Additionally, the reading of the file, and converting to List<String> is expected to be outside the function (in the main method) which cannot be done in ideone.com.

Here's the code in full:

import java.util.*;
import java.lang.*;
import java.io.*;
import java.util.stream.*;
import java.util.regex.Pattern;

class Morse
{

    private static final Map<String, String> morseAlphabet = new HashMap<>();

    static {
        morseAlphabet.put("", " ");
        morseAlphabet.put(".-", "A");
        morseAlphabet.put("-...", "B");
        morseAlphabet.put("-.-.", "C");
        morseAlphabet.put("-..", "D");
        morseAlphabet.put(".", "E");
        morseAlphabet.put("..-.", "F");
        morseAlphabet.put("--.", "G");
        morseAlphabet.put("....", "H");
        morseAlphabet.put("..", "I");
        morseAlphabet.put(".---", "J");
        morseAlphabet.put("-.-", "K");
        morseAlphabet.put(".-..", "L");
        morseAlphabet.put("--", "M");
        morseAlphabet.put("-.", "N");
        morseAlphabet.put("---", "O");
        morseAlphabet.put(".--.", "P");
        morseAlphabet.put("--.-", "Q");
        morseAlphabet.put(".-.", "R");
        morseAlphabet.put("...", "S");
        morseAlphabet.put("-", "T");
        morseAlphabet.put("..-", "U");
        morseAlphabet.put("...-", "V");
        morseAlphabet.put(".--", "W");
        morseAlphabet.put("-..-", "X");
        morseAlphabet.put("-.--", "Y");
        morseAlphabet.put("--..", "Z");
        morseAlphabet.put("-----", "0");
        morseAlphabet.put(".----", "1");
        morseAlphabet.put("..---", "2");
        morseAlphabet.put("...--", "3");
        morseAlphabet.put("....-", "4");
        morseAlphabet.put(".....", "5");
        morseAlphabet.put("-....", "6");
        morseAlphabet.put("--...", "7");
        morseAlphabet.put("---..", "8");
        morseAlphabet.put("----.", "9");
    }

    private static final Pattern charsplit = Pattern.compile(" ");

    private static final String decodeLine(String line) {
        return charsplit.splitAsStream(line)
                   .map(letter -> morseAlphabet.get(letter))
                   .collect(Collectors.joining(""));
    }

    public static final String decode(List<String> data) {
        String output = data.stream()
             .map(Morse::decodeLine)
             .collect(Collectors.joining("\n"));
        return output;
    }

    public static void main (String[] args) throws java.lang.Exception {
        String out = Morse.decode(Arrays.asList(
               "-.-. --- -.. .  .-. . ...- .. . .--",
               "..--- -. -..  -- --- -. .. - --- .-.",
               "..-. .-. --- --  -.-. --- -.. . . ...- .- .-.."));
        System.out.println(out);
    }

}


Ordering data.frames elegantly by factor I have a mock_Data:

structure(list(Sample = structure(c(29L, 31L, 21L, 25L, 15L,
17L, 13L, 27L, 23L), .Label = c("109d0", "109d2", "116d0", "116d2",
"125d0", "125d2", "136d0", "136d2", "138d0", "138d2", "141d0",
"141d2", "144d0", "144d2", "18d0", "18d2", "28d0", "28d2", "30d0",
"30d2", "39d0", "39d2", "4d0", "4d2", "70d0", "70d2", "73d0",
"73d2", "82d0", "82d2", "83d0", "83d2"), class = "factor"), Clinical = structure(c(1L,
2L, 1L, 2L, 1L, 2L, 1L, 1L, 2L), .Label = c("low faecal Salmonella shedding",
"persistent faecal Salmonella shedding"), class = "factor")), .Names = c("Sample",
"Clinical"), row.names = c(NA, 9L), class = "data.frame")

And I need to reorder it so that the rows with the same Clinical factor are together.

So far I generated this function :

reorder_by_factor <- function(data, factor){
  # Function to get the indices of an unamed rows
  flev <- levels(factor)
  ac <- function(data, factor, x){
    # Extract rownames
    rownames(data)[with(data, factor == x)]
  }
  ap <- sapply(flev, ac, data = data, factor = factor )
  unlist(ap)
}

that outputs the index of each factor ordered, and I can use it as mock_Data[reorder_by_factor(mock_Data, mock_Data$Clinical),]

I would like to be able to use it like lm with just reorder_by_factor(mock_Data, Clinical) and if possible make it more elegant and general, so it could take several factors and reorder them one, after the other. Also I would like to know which code would broke the function (aside from a data.frame with rownames)

I think the task you are attempting to accomplish (obtain indices to reorder a data frame by several factors, one after another) is already handled well by the built-in order function. Let's consider your sample data:

order(mock_Data$Clinical)
# [1] 1 3 5 7 8 2 4 6 9

As you can see, order is returning row numbers instead of row names; this yields an identical reordered data frame:

identical(mock_Data[reorder_by_factor(mock_Data, mock_Data$Clinical),],
          mock_Data[order(mock_Data$Clinical),])
# [1] TRUE

There are several other advantages of using the built-in order function:

It already smoothly handles reordering by multiple factors, one after another
It can reorder by a mix of factor and numeric variables
It is already implemented and distributed with base R, so you don't need to develop and test your own version of this function

Another advantage is that the built-in order function is more efficient than your implementation. Let's see on a resampled version of your dataset with 1 million rows:

set.seed(144)
big.dat <- mock_Data[sample(seq_len(nrow(mock_Data)), 1e6, replace=T),]
library(microbenchmark)
microbenchmark(reorder_by_factor(big.dat, big.dat$Clinical), order(big.dat$Clinical))
# Unit: milliseconds
#                                          expr      min       lq     mean   median        uq       max neval
#  reorder_by_factor(big.dat, big.dat$Clinical) 562.2527 744.4318 939.0831 898.6288 1054.0008 1678.1246   100
#                       order(big.dat$Clinical) 149.4750 153.2963 199.8178 160.5452  225.9082  459.5558   100

The built-in function runs about 5x faster.


Return all anagrams for a word matched against a list of words in a file The exercise description:

  Given a ‘words.txt’ file (attached) containing a newline-delimited
  list of dictionary words, please implement a small command line
  application capable to return all the anagrams from ‘words.txt’ for a
  given word. You can use any Scala library of your choice, no
  frameworks though.

Note: the file contains words that are sometimes Uppercased or have random upper/lowercasing. Checking for an anagram should not consider casing.

My solution:

Main.scala

import scala.io.Source
import scala.util.Using

object Main extends App {

  def isAnagram(word: String, anagram: String) : Boolean = word.toLowerCase.toSeq.sorted.unwrap == anagram.toLowerCase.toSeq.sorted.unwrap

  print("Type the word for which you wish to search anagrams: ")

  val word = scala.io.StdIn.readLine()

  Using(Source.fromResource("words.txt")){ source =>
    print("Anagrams found: ")
    val anagrams = source.getLines().filter(possibleAnagram => isAnagram(word, possibleAnagram)).toList
    println(anagrams match {
      case _ :: _ => anagrams.mkString(", ")
      case _ => "none"
    })
  }
}

MainTest.scala

import org.scalatest.FunSuite
import org.scalatest.prop.TableDrivenPropertyChecks

class MainTest extends FunSuite with TableDrivenPropertyChecks {
  test("anagram") {
    val parametersTable =
      Table(
        ("listen", "silent"),
        ("stressed", "desserts"),
        ("Dusty", "Study")
      )

    forAll (parametersTable) { (word: String, anagram: String) =>
      assert(Main.isAnagram(word, anagram) === true)
    }
  }

  test("not an anagram") {
    val parametersTable =
      Table(
        ("listen", "desserts"),
        ("stressed", "silent"),
        ("testing", "study")
      )

    forAll (parametersTable) { (word: String, anagram: String) =>
      assert(Main.isAnagram(word, anagram) === false)
    }
  }
}

Using scalatest library for writing the tests.

Stuff I considered when writing the code:

having data driven tests that cover all the basic corner cases
performance - the data is read from the file and processed in a streaming fashion
readability and conciseness - I tried to apply this as much as my Scala knowledge allowed me

Feedback I received from a third party:

Was told that this code is not good enough without getting any concrete explanation.

I'm going to review your Main code and let someone else address the Test code.

I'll start with a few minor issues.

Breaking up long lines often makes them easier to scan/parse/comprehend.

def isAnagram(word: String, anagram: String) : Boolean =
  word.toLowerCase.toSeq.sorted.unwrap == anagram.toLowerCase.toSeq.sorted.unwrap

Also, you're using .toSeq to get to the non-deprecated .sorted method, but there's no reason to .unwrap it to get back to a String. The == comparison will work on a WrappedString.

Using.apply() returns a Try[A], which your code ignores. If an exception is thrown (words.txt isn't where it should be?) then your app silently returns without reporting any errors. You might want to consider Using.resource() instead. It won't try to catch any exceptions.

Using pattern matching just to determine if anagrams is populated or not seems a bit bizarre. In fact, it doesn't need to be a List at all.

val anagrams = source.getLines()
                     .filter(isAnagram(word, _))
                     .mkString(", ")

if (anagrams.isEmpty) println("none")
else                  println(anagrams)

A big, big, problem with your algorithm is that the user-input, word, is processed for comparison (toLowerCase and toSeq and sorted) for every word in the dictionary. (*Ouch!*) It should only be done once.

But my biggest issue with the design is your choice of user experience (UX). Prompting for user input has many pitfalls. I won't go into the whole rigamarole about using the IO monad in an FP context, instead I'll just point out that simplifying the user I/O can greatly enhance the app's utility.

What if the user wants to count the number of anagrams for a given word? If your app had a small and simple usage profile (usage: anagrams <word>) then the task would be easy.

%%> anagrams Star | wc -w
5
%%>

From there you can imagine simple feature enhancements to make it more useful.

%%> anagrams Star bets ENDING
Star: arts, rats, tars, tsar
bets: best
ENDING: ginned
%%>