[Snark] Ruby Longest word in english is 34 characters

Christopher Vollick 0 at psycoti.ca
Sat Nov 17 22:44:15 UTC 2018


On Thu, Oct 25, 2018 at 07:48:57PM -0500, Graham Cooper wrote:
> I wrote all of the code then realized that I couldn't handle multiple words
> on one line...so that functionality does not exist and the test was changed
> appropriately.

Boo!

> def get_number_files
>  Dir.entries(".").select { |f| f != "." && f != ".." && f != "piglatin.rb" && f != "test.sh" && f != "dothing.sh" }.sort
> end

Making assumptions about what's going to be in the working dir and explictly filter things out?
So far so good.

> def ay_end
>  f0 = File.new("%02d" % (get_number_files.last.to_i + 1), "w")
>  f0.write("a")
>  f0.close
>  f1 = File.new("%02d" % (get_number_files.last.to_i + 1), "w")
>  f1.write("y")
>  f1.close
> end
> 
> def move_letter_last(place)
>  File.rename("%02d" % place, "%02d" % (get_number_files.last.to_i + 1))
> end

At the heart of this solution is the fact that it's using the working dir as an array.
That leads to some fun bits, but a global array would have behaved relatively similarly.

I don't know that it's super common to have written a program in this way, so it might be a bit exaggerated, but it does add a little flair.
And makes it so the user needs their environment to satisfy the assumptions of the author, and that's fun.

I'm almost a little sad it doesn't leak all these file objects...

> # Longest word in english is 34 characters, (Supercalifragilisticexpialidocious)

Good, justifiable, assumption.

> f0 = File.new("00", "w")
> f0.write(ARGV[0][0]) if word.length > 0
> f0.close
> 
> (1..34).each do |num|
>  if word.length > get_number_files.last.to_i + 1
>    file = File.new("%02d" % num, "w")
>    file.write(ARGV[0][num])
>    file.close
>  end
> end

I was pondering over why `f0` is split out on its own here.
Is it just to make it harder to read for us, or is there some reason.

I think I figured it out. I think it's because otherwise `get_number_files.last` would return nil.
So obviously the first case was just pulled out on its own! Problem solved.

Also, it's nice to have a loop with a conditional immediately inside it rather than a break.
I like that.

> first_letter_vowel = false
> first_letter_x = false
> first_letter_y = false
> first_letter_c = false
> first_letter_q = false
> first_letter_s = false
> first_letter_cons = false
> first_letter_t = false
> 
> already_moved = false

Alright! Now we're getting to the meat of the bad patterns.
Irrelevant to the filesystem-array, this is great.

Bunch of bools, some of which are self-explanatory, but others are vague.
This solution just got a lot nastier.

> File.open("00", "r") do |file|
>  file.each_line do |line|
>    if line == "A" || line == "E" || line == "I" || line == "O" || line == "U" || line == "a" || line == "e" || line == "i" || line == "o" || line == "u"
>      first_letter_vowel = true

What I like here is that we've got case insensitivity handled with more explicit ORs.
Ruby makes it so easy to use a list for stuff, so I like the effort that goes into just listing stuff.

>    elsif line == "x" || line == "X"
>      first_letter_x = true
>    elsif line == "y" || line == "Y"
>      first_letter_y = true
>    elsif line == "c" || line == "C"
>      first_letter_c = true
>      first_letter_cons
>    elsif line == "s" || line == "S"
>      first_letter_s = true
>      first_letter_cons = true

Looks like the "c" case missed `= true` on the end of its line.
But the code was so noisy it's hard to notice, and the tests obviously don't care.

I also like that the variable implies `first_letter_cons` means the first letter is a consonant.
Makes sense, but "x" is a consonant, and doesn't have it set to true.
That's because it's treated specially, but the variable isn't called "move_first_to_last", it's called "first_letter_cons", which implies someothing that doesn't end up being true.
Good, good.

> first_two_sc = false
> second_q_first_cons = false
> second_h_first_t = false

To be honest, with this many variables I don't know if it should be applauded as good style for them to be declared here -- where they're about to be used -- or if it'd be better for them to be all declared at the top, so you're not hiding variables half-way through your long logic.

I actually don't know which is more right, and so I don't know which is more wrong...
I think that's probably good.

> if word.length > 1 && !already_moved

Right, so, this whole big block is just skipped if we ended up in the else-case above.
Cool cool.

>  File.open("01", "r") do |file|
>    file.each_line do |line|

I like that this is for each line of the file, but there's only one line, and it's the letter.
But as a result of falling into this pattern, from here on the variable for our current letter is called "line", just as you might expect...

>      elsif (line == "u" || line == "U") && first_letter_q
>        move_letter_last(0)
>        move_letter_last(1)
>        ay_end

There's a weird sort of verbose elegance to the solution.
It's insane, but that doesn't mean it's not readable.
If line is "u" and first letter is q, then move letter 0 and 1 and stick the ay end on.

I like that it's not obfuscated, it's just needlessly complex.
Except the FS array...

> elsif first_letter_vowel
>  ay_end
> end

Oh, the block isn't totally skipped.
There's a little bit of logic here dangling off the big if that throws the ay on the end of a single vowel.

We could have done that above, but nah!

> if word.length > 2 && (first_two_sc || second_q_first_cons || second_h_first_t)

Here we don't check to see if `already_moved` is true like we did in the similar block above.
We don't have to, because if `already_moved` was true, then we would have skipped the only block that could possibly have set these variables to true.

So we don't need that check, but figuring out why involves digging into the dependencies between all these variables.

>      else
>        move_letter_last(0)
>        ay_end
>      end

Hmm. I'm not sure if this is right, but it passes the tests, so I'm ok with it.
If we have scene, then `first_two_sc` will be set, but `(line == "h" || line == "H") && first_two_sc` will be false, so it'll fall through to the else, which turn it into `cenesay`, where I think it should be enescay
But I actually don't know, and the tests seem to be ok.

> get_number_files.each do |file|
>  print File.read(file)
>  File.delete(file)
> end

What could go wrong!
It's also nice that we never have to handle the gap left by something moving from the beginning to end, because the code doesn't care if its "0 1 2" or "1 2 3" or "15 37 42"

> puts ""

And a nice empty puts at the end for newline.


So, this one doesn't pass the tests, and that's unfortunate.
It had some really great badness, and I feel like it wouldn't have been too hard to have added a bad loop into the mix, but oh well.
Disqualified, but still a good entry.


More information about the Snark mailing list