Hi guys. After our meeting at Lille, I did a pass of SmallLint over Fuel
project. I have to say that I enjoyed the tool and that. I love the
manifiesto idea. I love to store metada and to be able to reject rules and
scenarios to avoid having false positives again in the future. In addition,
it looks like I had less false positive than with other tools. However,
there were a couple of rules that I don't understand or may suggest
something:
1) First, apart form showing the package between parenthesis, I would also
show the protocol (category). Because some rules play with that, so it is
handy if you can directly see them there, otherwise you need to browse the
code.
2) RBContainsRule is not clear for me. *" 'Checks for the common code
fragment: "(aCollection detect: [:each | ''some condition''] ifNone: [nil])
~= nil". contains: can simplify this code to "aCollection contains: [:each
| ''some condition'']". Not only is the contains: variant shorter, it
better signifies what the code is doing.'"*
That is not very true. #contains answers true or false, while
detect:ifNone:[] answers whatever, and in your example it is nil. So, the
answer will be different and hence it is not the same. Ok, in your example,
you compare after with == nil... but... I don't really see a use case. For
example, in my case it detected:
classNamed: className
^ (migrations
detect: [:m | m sourceClassName = className ]
ifNone: [ ^ self globalClassNamed: className ])
targetClass.
3) RBLawOfDemeterRule: maybe you can exclude string concatenation? (#,)
because it detected a lot of messages like this:
signalWith: aGlobalName and: aSelector
^ self signal: 'Method ', aGlobalName, '>>#', aSelector, ' not found.'
Also, what about excluding messages like #asXXX. For example, here it
detected "self signature asByteArray". Is that completly breaking the law
of demeter? mmm I don't think so.
verifySignatureFrom: aDecoder
| streamSignature |
streamSignature := ByteArray new: self signature size.
aDecoder nextEncodedBytesInto: streamSignature.
(self signature asByteArray = streamSignature) ifFalse: [
FLBadSignature
signalCurrentSignature: self signature
streamSignature: streamSignature ].
4) RBSentNotImplementedRule: I didn't understand why it has selected:
largeIdentityHash
<primitive: 75>
self primitiveFailed
both messages are implemented :(
5) RBClassInstVarNotInitializedRule could be smarter and search not only
for #initialize but for #initializeXXX. Lots of times, I initialize my
objects with data, using a particular nitialize method, such as:
initializeWithBehaviors: someBehaviors extensionMethods:
someExtensionMethods
so the rule says there is no initialize...but I have that one ;)
That's all the feedback so far. It helped me to find some things to improve
so thanks!
--
Mariano
http://marianopeck.wordpress.com
