[Snark] Ruby Longest word in english is 34 characters

Stephen Paul Weber singpolyma at singpolyma.net
Fri Nov 9 02:29:13 UTC 2018


This one fails the tests, but only one of them, and it does solve the 
problem and is pretty bad so I let it in this time.  Though even a jank 
"just make it work" when the submitter realised the final test existed would 
have been nice.

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

A lot of this submission comes down to making bad assumptions.  This is the 
first one we see -- meant to get the "number files" created below, this 
actually gets *all* the files minus a known set.  So the main badness IMO is 
this subtle bug.

This line also encodes a list of options as hard-coded comparisons, making 
the line longer and harder to read or edit that neccessary.

Oh, also putting temp files in the working directory FTW.

>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

Here we really begin to suspect the second crazy thing about this program.  
What are these files for?  This method seems to exist in order to add "ay" 
to the end of the word... but it is doing that by creating two files? What 
is happening?!

A few bad things specifically here: getting the index to create by first 
listing all the files, not using block-style open and so having to manually 
close the handle.

>def move_letter_last(place)
> File.rename("%02d" % place, "%02d" % (get_number_files.last.to_i + 1))
>end

So to... move a letter... we... move a file.  Oh my.  Also, we just leave a 
hole where that letter was?  Again listing all files to find out where we 
are, great misuse of global state.

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

AAAAAHHHHHHHH!

>f0 = File.new("00", "w")
>f0.write(ARGV[0][0]) if word.length > 0
>f0.close

AAAAAAAAHHHHHHHHH!!!

>(1..34).each do |num|

Why are we iterating over 1-34 instead of the actual length of the word? Why 
did we not start at zero to avoid the extra special case above?

> if word.length > get_number_files.last.to_i + 1

Why are we wrapping the whole loop in an if instead of modifying the loop 
condition... or at least using break?  Wait, does this list all the files 
EVERY TIME THROUGH THE LOOP just to find out if we made them all yet?

>   file = File.new("%02d" % num, "w")
>   file.write(ARGV[0][num])
>   file.close

Wait, why am I even worried about that.  WHY ARE WE WRITING EACH LETTER OF 
THE WORD TO ITS OWN FILE?!

I still have the energy for nits, so also this code should use block-style 
open to avoid explicit close... if it should exist at all...

>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

NOOOOOOOOOOOOOOOOOOOOOOOO! NO NO NO NO NO NO NO!

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

Each... line?  In our file that we know for a fact contains only one letter 
and *no* lines?

>   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

Oh my.

>   elsif line == "y" || line == "Y"
>     first_letter_y = true

Oh dear.

>   else
>     first_letter_cons = true
>     move_letter_last(0)
>     ay_end
>     already_moved = true

`aleady_moved = true` -- this is just an epic overdo of the classic "I heard 
goto is bad so I use flags for every control flow".  Also, this moves the 00 
file to the end, and does not create a new 00 file.  I hope nothing after 
this point wants a first letter to exist...

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

Oh, goody.  We're not done with these yet.

>if word.length > 1 && !already_moved
> File.open("01", "r") do |file|
>   file.each_line do |line|

There's that each_line again.  Also, open "01" ?  Are we going to open each 
of the 34 files one by one here?

>     if (line == "r" || line == "R") && first_letter_x

I do like this constant, terminal blindness to the idea that letters have 
case.  It's two different letters don't you know!  The only way that could 
be worse would be by having a different elsif branch for each case.

I also like that because of the flag variable set in a whole other above 
block, this effectively obscures that we're checking for "starts with xr".

>if word.length > 2 && (first_two_sc || second_q_first_cons || second_h_first_t)
> File.open("02", "r") do |file|
>   file.each_line do |line|

Here we go.

Wait... this doesn't check already_moved.  It will happen to work because 
the other flags won't be set if the above step got skipped... but so it just 
works by accident.  Excellent.

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

Oh, apparently we're done now.  At least there's only three hardcoded file 
checks :P  Nothing happens to care about letter position after these and 
letters are checked in order, so the move-to-end leaving holes won't break.  
More working by accident!

>puts ""

This is just a bonus fun one, because the `""` argument is entirely 
unneccesary.

I like this one, I think it's a contender.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://snark.badcode.rocks/pipermail/snark/attachments/20181108/200ce1dc/attachment.sig>


More information about the Snark mailing list