Thanks for feedback :) Indeed, this is a nice case to learn from. That piece of code was written when I just started working with pharo after coming from Java world.
Purpose of that catch, as mentioned by Nicolai, is indeed to ignore late initialized objects. For example, imagine a Morph with height that is equal to owner's height. In this case one would write in #initialize: initialize self height: [ :morph | morph owner height ]. After setting an object as a new height, it gets evaluated and in case of example before throws DNU because owner is obviously still nil. Trying to improve overall readability I decided to ignore all errors in that kind of blocks instead of putting ifNil:ifNotNil: in hundred places. Also, I could not imagine that Warning is actually an Exception and exception (in its common sense) is an Error. However, I had in mind that later that kind of code smell should be replaced at least by custom error handler, like we have now in spotter that catches all errors allowing not buggy processors to do their work. Btw, in first versions of spotter it was the same: it catched exception instead of error but more experienced colleges suggested to not do it like that. Nevertheless, later I realized that blocks are evil, and instead of trying to work with nils it's much easier and cleaner to just put an assertion. As a hotfix, I will replace Exception to Error tomorrow. (bad, but still...) Don't hesitate to post code smells, at least we can make jokes of them :) Cheers Alex On Mar 30, 2016 1:34 PM, "Nicolai Hess" <nicolaih...@gmail.com> wrote: > Please don't do this: > > updateHeight > "no need to care about height, when it's logic is not customized" > self layout isHeightCustom ifFalse: [ ^ self ]. > [ self bounds: (self brickBounds withHeight: self customHeight) ] > on: Exception > do: [ "just skip and do nothing" ] > > This makes debugging GLM/Brick ui/layout code with "self haltOnce" > impossible. > see > GLMBrickGeometryTrait>>#updateHeight > GLMBrickGeometryTrait>>#updateWidth > > And if you log out the raised exception, you see some calls to > not initialized fonts and a ZeroDevide and some more errors. > The above catch, catches and hides wrong / to late initialized objects. >
