This is one of the stickers for the second edition of Rails Girls Vratsa which was held yesterday. Let's explore some of the bug proposals submitted by the Bulgarian QA group:
- sad() == true is ugly
- sad() is not very nice, better make it if(isSad())
- use sadStop(), and even better - stopSad()
- there is an extra space character in beAwesome( )
- the last curly bracket needs to be on a new line
My friend Lu describes what I would call style issues. The style he refers to
is mostly Java oriented, especially with naming things. In Ruby we would probably
sad? instead of
isSad. Style is important and there are many tools
to help us with that this will not cause a functional problem! While I'm at it let me say
the curly brackets are not the problem either. They are not valid in Ruby this is
a pseudo-code and they also fall in the style category.
The next interesting proposal comes from Tsveta Krasteva. She examines the possibility
sad() returning an object or nil instead of boolean value. Her first question was
will the if statement still work, and the answer is yes. In Ruby everything is an object
and every object can be compared to
Alan Skorkin's blog
post on the subject.
Then Tsveta says the answer is to use
sad().stop() with the warning that it may return
nil. In this context the
sad() method returns on object indicating that the person
is feeling sad. If the method returns nil then the person is feeling OK.
class Csad def stop() print("stop\n"); end end def sad() print("sad\n"); Csad.new(); end def beAwesome() print("beAwesome\n"); end # notice == true was removed if(sad()) print("Yes, I am sad\n"); sad.stop(); beAwesome( ); end
While this is coming closer to a functioning solution something about it is bugging me.
In the if statement the developer has typed more characters than required (
This sounds to me unlikely but is possible with less experienced developers.
The other issue is that we are using an object (of
class Csad) to represent an internal
state in the system under test. There is one method to return the state (
another one to alter the state (
Csad.stop()). The two methods don't operate on
the same object! Not a very strong OOP design. On top of that we have to call the
method twice, first time in the if statement, the second time in the body of the
if statement, which may have unwanted side effects. It is best to assign the return
value to some variable instead.
IMO if we are to use this OOP approach the code should look something like:
class Person def sad?() end def stopBeingSad() end def beAwesome() end end p = Person.new if p.sad? p.stopBeingSad p.beAwesome end
Let me return back to assuming we don't use classes here.
The first obvious mistake is the space in
sad stop(); first spotted by Peter Sabev*.
His proposal, backed by others is to use
sad.stop(). However they
didn't use my hint asking what is the return value of
sad() returns boolean then we'll get
undefined method 'stop' for true:TrueClass (NoMethodError)!
Same thing if
sad() returns nil, although we skip the if block in this case.
In Ruby we are allowed to skip parentheses when calling a method, like I've shown
above. If we ignore this fact for a second, then
sad?.stop() will mean execute the
stop() which is a member of the
sad? variable, which is of type method!
Again, methods don't have an attribute named
The last two paragraphs are the semantic/functional mistake I see in this code. The only way for it to work is to use an OOP variant which is further away from what the existing clues give us.
Note: The variant
sad? stop() is syntactically correct. This means call the function
with parameter the result of calling the method
stop(), which depending on the outer scope of this program may or may not
be correct (e.g.
stop is defined,
sad? accepts optional parameters,
Thanks for reading and happy testing!