Thanks for the feedback On Apr 21, 2012, at 2:00 PM, Mariano Martinez Peck wrote:
> 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 >
