[Snark] Ruby is so slow

Stephen Paul Weber singpolyma at singpolyma.net
Fri Nov 9 04:03:23 UTC 2018


>require './english.rb'
>require './pig.rb'
>require './latin.rb'

I was actually surprised to find that this works.  Ruby specifically has 
require_relative for this kind of thing.  Also, you're not supposed to 
include the ".rb" in the require line, usually.

>words = ARGV[0].split(' ')
>output = []
>words.each do |word|
> dictionary = English.new
> place = if dictionary.is_english?(word)
>   dictionary.data.find_index(dictionary.the_word)
> end
> latin = Latin.new
> pig = Pig.new(latin)

I have a bad feeling about this... Why do we create a new English, Latin, 
and Pig every loop even though they take no arguments from the loop context?  
Why do we check if the dictionary has this word as English before we...  
search its data manually for the word?  Seems redundant, and also like a 
method that should exist on the dictionary itself.  Wait... we search the 
dictionary based on an attribute... that is set by our checking function?  
Oh no...

> output << pig.data[place]

The old antipattern "push stuff onto an array in a loop".

>end
>puts output.join(' ')

Why didn't we just print as we go if we're going to do this right on the 
next line?

> def initialize
>   @data = []
>   load_data("english.data")

Always good to have another method that implements 100% of what this method 
should do.

> def is_english?(word)
>   is_english = 'not yet'

Because booleans are too easy.

>   letters = word.split('')

word.chars is newer, let's not judge to harshly ;)

>   letters.each do |letter|
>     data.each do |english|
>       if english.include?(letter)
>         if is_this_the_word?(letters.join(''), english)

Good optimization: only check the word if the letter is in it :P  We're 
going through the whole dictionary FOR EACH LETTER and then just doing some 
insulting wastefulness on top.

>           is_english = 'yes'

Don't even break once we find the answer, just set a flag to a string and 
keep running the loop through everything to waste as much time as possible.

>   is_english == 'yes'

Convert to a bool because at least our public API is not insane.

> private

Bah. private

> def is_this_the_word?(this, word)
>   @the_word = word if this == word
>   this == word
> end

Ask a question, get an instance variable!

> def load_data(data)
>   file = File.new(data, "r")
>   while (line = file.gets)
>     @data << line.downcase.strip

More loop-and-push antipattern.

>   end
>   file.close

Also, manual file close antipattern.

>class Latin < English
> attr_accessor :data
> attr_accessor :the_word
>
> def initialize
>   @data = []
>   load_data("english.data")
> end

Always good to override your superclass with an exact copy of what it 
already does.

Also, what is any of this for?

> def is_english?(word)
>   'no'
> end

This is amazing, because the API expects bool, but ruby's truthiness rules 
mean this will evaluate to true.

>class Pig
> attr_accessor :data
>
> def initialize(latins)
>   @data = []
>	  roll_in_mud(latins)
> end

More "made a private method to implement the total body of another method" 
antipattern.

> private

private. 2 points

> def roll_in_mud(latins)

See, it's a joke, because pigs!

>   latins.data.each do |latin|

If latins is a Latin then latins.data is a list of English words, so this 
latin is a word in English.  Not confusing at all.

>     piglatin = ''
>     @snorts = 0
>     @squeals = 0

More jokes.  Jokes are funny, and you didn't need to know what these 
variables were for anyway.  Also, why are these instance variables?

>     if latin == 'yellow'
>       piglatin = 'ellowyay'

I guess the 'y' case was too hard.

>     elsif letters.length > 2 && letters[0] == 'x' && letters[1] == 'r'
>       piglatin = latin + 'ay'

We initialized `piglatin` to empty string, but just overwrite it everywhere.  
Good, good.

>       piglatin = letters[letters.find_index('u')+1..-1].join('') + letters[0..letters.find_index('u')].join('') + 'ay'

Use `letters` because we're too cool for string slices.

>       letters.each_with_index do |l, i|
>         snort if ['a', 'e', 'i', 'o', 'u', 'y'].include?(l)
>         squeal unless ['a', 'e', 'i', 'o', 'u', 'y'].include?(l)

So we *always* either snort or squeal.  And these are instance variables so 
we can use a fun method to do the increment for no reason.  Hooray.

>     @data << piglatin

More push-in-a-loop

>== english.data ==

A list of every word in English.  Essential to any pig latin implementation.

There's some nice stuff in here.  The main target seems to be inefficiency, 
and boy does it deliver, taking *minutes* to pass the tests where most other 
submissions take under a second.
-------------- 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/b4eb1e77/attachment.sig>


More information about the Snark mailing list