On 26.01.2011 08:13, HwaJong Oh wrote:
My last package is Balloon-hjo.72.mcz.
It is small testing& refactoring on Ballloon supporting collections.
I wonder this type of work is acceptable?
Best Regards
Refactorings and more tests are certainly welcome!
Here's a few comments:
While I like keeping common functionality in one place, I'm not sure
creating a new helper object for each at:/at:put: access is a good idea.
Nor keeping the helper cached to avoid this. Have you considered using a
Trait instead? To me it seems like a good use-case for one.
To the tests:
- setUp uses at:put:. testAt asserts the values set in setUp. Thus, is
it not really just another at:put: test? Does it test something not
covered by the other at:put: test(s)?
- testAtPutLargeInteger uses a static integer. This is not ideal, as the
range of SmallInteger depends on platform (I realize though, that it is
large enough for it not to fail on 64bit as well).
Using f.ex SmallInteger maxVal + staticInteger is more intention revealing.
Although, max/minVal actually returns a constant at the moment rather
than a platform-specific value, and thus would fail on 64bit. I'd
consider this a bug which it'd be nice if a test exposed :)
- testWriteXYEndianOn uses 256*256*256. etc. Probably down to
preference, but I think they'd looks nicer using the bitShift: (or #<<
if you prefer less parenthesis) operator.
Cheers,
Henry
