Basic challenge (simple): Write a script that produces all possible 4-digit numbers (0000...9999), then put them into a random order and save the output into a text file. Make sure you include leading zeroes where necessary and make also sure that each number is on a separate line in the text file.
Bonus challenge (for daring programmers only): Do the basic challenge and then do the following: Call the generated file leaked_pins.txt
, and email it to one of your co-workers. In the description of the email, say that all the PIN numbers have been leaked, and they should be cautious and check, whether their PIN is contained in the file or not.
Share your code below. If any of you tries the bonus challenge, I am happy to hear any stories :)
Top comments (47)
Bash:
for i in {0..9999}; do printf "%04d\n" $i; done | sort -R > pins.txt
Using
shuf
utility for shuffling:for i in {0..9999}; do printf "%.4d\n" $i; done | shuf > pins.txt
The two scripts are equivalent as the
man sort
page tells that-R
option usesshuf
.I don't have
sort -R
orshuf
(I'm on OS X). I did not know you could do the dot precision thing with integers! My solution was basically the same, but I did"%04d\n"
You can get
gshuf
if you install the GNUcoreutils
through homebrew or ports.Ahh, there we go _^
Shorter:
seq -w 9999 | shuf > pins.txt
Just wanted to share my script :p
Only gets all the numbers from 0000 to 0999.
edit: Simple fix though I think - should say Array(10000) not Array(1000)
Hey good catch, thanks.
Hey, this is nice. Can you please explain the function you passed to
sort()
?Sure, we were supposed to sort randomly and
sort()
expects a function that compares two values and returns>0
if greater0
if equal and<0
if lesser, so,For the random order, I used
Math.random
that returns a value between 0 and 1To be sure that it can return a negative number I multiplied the random times
number * 2
so:Math.random < 0.5
it will be positiveMath.random == 0.5
it will be zeroMath.random > 0.5
it will be negativeHowever, since we have three possible cases it could be better to do something like:
That way there is a 33.33% of probability for each case.
I hope you don't mind me answering on Pichardo's behalf (or Pichardo).
Sort iterates over consecutive pairs through the array and takes their differences. If a negative value returns, it knows the previous value comes before it, if positive, then after, and it sorts the array accordingly. But we don't want that to happen here as we're dealing with strings, e.g. we don't want the string, "39" to come after the string, "220" but alphabetically, they ought to just like "alphabet" comes before "bat". Since sort only knows what to do based on the sign it receives, if we pass the two indices as parameters to a function and have it do their difference numerically, it sorts everything as expected. And since we don't actually want a sort in this case, but want things random, if we only pass the first of the two indices of each pair and compare it to a random value, we get things randomly "sorted".
An even simpler way to achieve the same result would be to just do .sort(number=>.5 - Math.random()).
That's right
0.5 - Math.random()
would've worked as wellWhat about an OO version of the possible solution in Ruby?
gist.github.com/vgarro/1adbc5e8df8...
Hi, Victor! 😊
I'm super ambivalent about this, lol! On one hand, I think the right solution for the problem is the procedural one-liner
File.write "leaked_pins.txt", (0..9999).map { |n| "%04d\n" % n }.shuffle.join
, but on the other hand, I think it's incredibly important to be able to do what you're doing here.So, since you took the time to write it, I'll take the time to review and refactor it. Hopefully I can be helpful 😊 Also, not everyone is going to agree with me, so I'll try to offer my reasoning for each opinion. My refactored code is here.
#process
. So now we would call it like this:ExcerciseOrquestrator.new('leaked_pins.txt', 4).process
. A natural evolution from that point isExcerciseOrquestrator.process('leaked_pins.txt', 4)
, which allows you to abstract the implementation to the point that callers don't even need to know whether an object is created to process the task or not.#call
, rather than#process
ExcerciseOrquestrator
, I'd move the reader into the private section, and onNumberGenerator
, I'd move#digits
,#max_number
, and#first_number
into the private section. Treat them as implementation details until there is a need to expose them (and even then, question the need before you oblige it).attr_reader :output_file_name
into the private section, you can turn it into anattr_accessor
Presumably the reason you did reader over accessor is because you don't want anyone to be able to call the setter from the outside, which is a good intuition, making it private accomplishes the same thing. Once you have theattr_accessor
, you can change the@output_file_name = ...
toself.output_file_name = ...
, the advantage of this is that it will explode helpfully if they ever get out of alignment. Eg if you're refactoring you might change the name, if you forget to change it where you set the value, then it will just set the wrong ivar, but if you use the setter, you'll get a helpfulNoMethodError
.file.write("#{row}\n")
can becomefile.puts row
, and sinceputs
will take an array of things to puts, you can remove the iterationopen_file { |file| file.puts output }
#write_to_file
takes an argument namedoutput
, which I would expect to be a string, given the name and the responsibilities of this class. A better name might bepins
,numbers
,rows
,elements
, orlines
, which makes it clearer that it's a collection. In your example, you call each element arow
, so renamingoutput
torows
is a pretty reasonable choice. Generally, I'd start withpins
orpin_numbers
, because it's the most concrete, so it's the easiest to understand, and then I'd move to something more abstract, as the class becomes more abstract (ie as the numbers aren't always pins, or as the things to write aren't always numbers).do
ondo_generate
do_generate
always returns a list, so no need for(arr || [])
, and thus no need for the lvar, nowNumberGenerator#process
can be implemented asdo_generate.shuffle
#do_generate
, usemap
rather thaneach_with_object
, eg(first_number..max_number).map { |digit| digit.to_s.rjust(digits, '0') }
#max_number
to#last_number
, just to to align it with#first_number
#do_generate
, the element is nameddigit
, but that implies it's 0-9, I'd rename this ton
ornumber
(n
is totally fine here, eveni
would be okay, partly because it's incredibly common to use those names for this kind of thing, so people understand it, and partly because we're iterating overfirst_number..max_number
, so clearly it's a number)NumberGenerator
, the worddigits
could imply a collection of digits to generate from, so I'd rename it tonum_digits
to make it clear that we're expecting an int.#open_file
, it passes a block which then yields to the method's block. So, it's just sort of passing the argument through. We can instead explicitly receive the blockdef open_file(&block)
and then pass that, rather than our own block literal:File.open(output_file_name, 'w', &block)
#max_number
, be careful to put matching whitespace on both sides of operators. Here, it's not an issue, but in other situations it can be read as "negative one", which can change how it parses. Example:[10, 20, 30].first + 1 # => 11
vs[10, 20, 30].first +1 # => [10]
Hi Josh!
I have to say, I was really impressed about the time you took to get all those comments together.
It really helps me, it really helps the community reading this comments.
In most sense, I completely agree with almost all your comments
it writes to the wrong filename.
That's quite not 100% accurate, as the original excersice (without the bonus) tells: "Then put them into a random order and save the output into a text file." .. as you can see, no file specified.Agree
Agree! (and consider it a mental note tattooed to my forehead!)
Agree as well, I guess I had the intention to move those two methods down the private section, but never did. Clumsy Victor!
attr_reader
vsattr_accessor
.. I have always liked the idea to have inmutable objects rather than allowing a change outside of the Object itself. It doesn't feel right. So, unless there's an explicit need to change the object and do recalculation, I'd stick withattr_reader
do_generage
"There are only two hard things in Computer Science: cache invalidation and naming things". -- Phil KarltonAgree
Agree
Agree
...
Overall, I really liked your approach.
One thing I could eventually do better, is follow Ruby style guide github.com/bbatsov/ruby-style-guide like single line method definitions, or somethimes using parenthesis and some other times not.
On any case, Thank you Josh! Thank you for your time and for your refactor.
Ahh, yes, this is fair. The filename was part of the "bonus challenge", which hopefully no one actually did :P
Yeah, I'm wary of mutability, too (hence moving the arg from
process
toinitialize
). The purpose for theaccessor
here is so that when we initialize it, we can doself.var = ...
rather than@var = ...
, which I prefer because if the names become misaligned, then it will error where we try to set it, rather than where we do something after accessing it (or worse, if it doesn't error at that point, b/cnil
is a valid value, and instead it blows up elsewhere in the program, or we just get really unexpected results). If we were calling the setter more than once, I'd advocate using a local var instead of an instance var. I guess I'm thinking of it as a "one-time-use" setter.You know, the more I write Ruby, the more I contemplate making my own
attr_*
methods :P In this case, the way I use it, a better definition would be something like this.I had no issue with any of your syntactic choices (other than the
-1
having mismatched whitespace). Looking at mine, there were a few weird ones, I think that's b/c I was inlining things so that my code samples in the comments would be inline, because the markdown processor doesn't seem to nest code blocks inside of lists. But, I initially wrote all my methods as multiline, and it was really just an error from trying to juggle the formatting requirements of the different places the data existed (my editor, the gist, within the comments).I'm going to need to see the full test suite for this too 😉
Serious question - How do you test randomness of an output?
One way is by controlling the seeder if possible, seeding it with same value every time should give you the same value every time.
There are some languages where you can instruct the test suite to run a test e.g. a hundred times and if those don't break the code you can be fairly confident (though not sure) that the code does what's intended. If your code takes input these tests usually test boundary conditions, maybe max and min values and a number of random values. If I remember correctly I saw it when playing around with Haskell.
I'd check the things you can know. Such as, if you ran the same method twice then it should result in two arrays of the same size that contain all the same things, but that aren't equal, i.e. the order is different.
I tried another thing recently where I needed to create a random string as output. I used dependency injection for the randomiser function with a default to the built in language version. Then in my test, I used a deterministic object for the randomiser function, such that I knew what the output was. I wasn't implementing the actual randomiser function, so testing it is not necessary, but testing that I'd call the right method on it and it returned the thing I expected satisfied me at the time.
Just some ideas.
But wouldn't this fail at times?
f.e. I have 3 elements in my array,
[1, 2, 3]
. Now the shuffle method only has six outcomes here, so there is a real possibility that the tests might randomly fail even if the code is correct.Dependency injection seems like a better approach if you want to test it, but it makes code not as nice to read.
Trade-offs I suppose.
Aye, that's true. I was kind of thinking about it in terms of generating an array of 10000 items!
Writing testable code is a trade off, but probably worthwhile in the long run. Dependency injection itself makes for more and more flexible code without a great deal of extra cognitive overload, so I am a big fan of it at the moment.
This was a great talk from Sandy Metz that drove towards this point for a different reason too. I recommend a watch!
Sharing my Ruby version of this script. Feel free to use to attempt the bonus challenge!
Here's my python approach:
That is awesome. Didn't know you could use an
f.write()
inside a list comprehension :D Real nice.That's with T-SQL using a Tally table
I'll see you and raise you Postgres :)
Works like a charm in MSSQL if
random()
replaced withnewid()
! :)PowerShell
I didn't know this for a long time, but you can use strings to build ranges in Ruby as well, avoiding the need for any fancy padding with zeros!
F#
Clojure?
Some comments may only be visible to logged-in visitors. Sign in to view all comments.