HiveBrain v1.2.0
Get Started
← Back to all entries
patternMinor

"Curious Numbers" (HackerRank PE 34)

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
hackerrankcuriousnumbers

Problem

I was inspired by a certain recent question to try my hand at this challenge:

Project Euler #34: Digit factorials


\$19!\$ is a curious number, as \$1!+9!=1+362880=362881\$ is divisible
by \$19\$.


Find the sum of all numbers below \$N\$ which divide the sum of the
factorial of their digits. Note: as \$1!,2!,\cdots,9!\$ are not sums, so they are not included.


Input Format: Input contains an integer \$N\$


Output Format: Print the answer corresponding to the test case.


Constraints: \$10^1 \le N \le 10^5\$


Sample Input

20




Sample Output

19


The code works great, with the only unfortunate problem being outside of my control, but is preventing me from completing the "official" HackerRank challenge: STDIN doesn't work (yet) with clojure. I Googled this and Stack Overflowed that, and seems everyone is having issues (even simple STDIN on HackerRank website using Clojure language errors out) so I know it's not just me. The function that directly addresses the challenge is sum-all-curious-numbers-up-to.

I wrote a whole bunch of tests to ensure that everything was working correctly. I also included some benchmark tests at the bottom of the post, as I am also interested in improving performance, if there is an idiomatic way to do so.

This is my first "real" challenge using FP and so I would like criticism on any and all aspects of the code, including algorithms/logic, style and tests. I also have a few specific areas of focus that I would like addressed in more general terms:

-
Is it idiomatic FP to throw exceptions, such as IllegalArgumentException which I often do here, or would it make more sense to return a falsey value when functions are passed an argument they are not designed to handle?

-
Specifically with explode-num-to-digits (and consequently test-explode-num-to-digits) is that function trying to handle too many edge cases? I thought it would be "neat" to be able to accept numbers as strin

Solution

I don't really know Clojure, but I enjoyed reading this code.
It's especially great that you've included unit tests.
I have a few nitpicks though.

According to your tests, the exponent of 0 is 0:

(t/is (= 0 (exponent 0 0)))


But by math, \$0^0\$ should be 1.

Somewhat similarly, factorial is usually not defined for negative numbers, but you define \$(-n)!\$ as \$-(n!)\$. Not a big deal though.

Something else that I find strange is that is-curious-number returns two kinds of types, boolean or numeric:

(t/is (false? (is-curious-number 10)))
  (t/is (= 19 (is-curious-number 19)))


I'm wondering if this is common practice in Clojure for some reason, because normally it's not a great idea. It would seem to make sense to return boolean, as the function name implies.



  • Specifically with explode-num-to-digits (and consequently test-explode-num-to-digits) is that function trying to handle too many edge cases? I thought it would be "neat" to be able to accept numbers as strings and parse those the same as a regular number, but does it even make sense for this function to handle such arguments?




I think it's a common trap when programmers try to do something "neat", that they don't really need, and then get into trouble for it.
The challenge states that the input is a number \$N\$.
It won't make much sense passing anything else,
it only adds the tedious overhead of numeric validation.
It's not necessary to support string inputs, so don't.

Of course, when reading from STDIN, you do need to convert strings to numbers at some point, but you should do that only once, as close to the point of input as possible, and let the inner layers of your solution safely assume that they will receive valid numeric values.

For example there's no need for exponent to handle non-numeric arguments. It's used at lower levels of your solution, and should be able to expect to be protected by the higher levels already.



  • Is it idiomatic FP to throw exceptions, such as IllegalArgumentException which I often do here, or would it make more sense to return a falsey value when functions are passed an argument they are not designed to handle?




By my answer to your second question, this question simply vanishes (as far as this Project Euler example is concerned). In addition, input validation is generally redundant in online challenges. In real life applications the code needs to be robust and vigilant when handling any kind of untrusted input, but this is generally not the case in online challenges.

Code Snippets

(t/is (= 0 (exponent 0 0)))
(t/is (false? (is-curious-number 10)))
  (t/is (= 19 (is-curious-number 19)))

Context

StackExchange Code Review Q#122672, answer score: 4

Revisions (0)

No revisions yet.