If there's an error in the expression that is assigned to file, and the stack unwound, file won't have been assigned and the close will get sent to nil. The assignment to file should precede the block. Sent from my iPhone
On Aug 31, 2015, at 5:03 AM, Nicolai Hess <nicolaihess@web.de> wrote:
Anyone? We may remove this rule if no one understands for what this is good for. (if anyone knows, please comment that class).
2014-12-14 11:22 GMT+01:00 Nicolai Hess <nicolaihess@web.de>:
2014-11-30 20:54 GMT+01:00 stepharo <stepharo@free.fr>:
thanks we should open a bug entry to improve the class comment of this rule. Any taker?
Stef
Le 30/11/14 12:10, Thierry Goubier a écrit :
done 14618 Add a comment describing RBFileBlocksRule
now I just need to have a good description for this rule. Please, can someone outline the discussion on this thread: - describe why code matching this rule is considered back practice - describe how the code could be rewritten.
nicolai
Le 30/11/2014 12:01, Max Leske a écrit :
On 30.11.2014, at 11:18, Thierry Goubier <thierry.goubier@gmail.com> wrote:
Le 30/11/2014 11:06, Max Leske a écrit :
The code would look like this:
[ | file | file := StandardFileStream forceNewFileNamed: âfooâ. file nextPutAll: âbarâ ] ensure: [ file close ].
Why that is considered bad practice however, I canât tell.
This one would fail because file is a temp to the block (and not visible to the ensure block).
If it werenât visible, wouldnât the compiler raise an exception?
In your example, yes.
But the rule matched:
| file | [ file := StandardFileStream forceNewFileNamed: âfooâ. file nextPutAll: âbarâ ] ensure: [ file close ].
(Hum: wouldn't the compiler complain of file might be nil in the ensure block?)
Another problem is that, if you have a failure in forceNewFileNamed:, file is nil and the ensure: fail as well.
True, but thatâs a problem this rule doesnât really cover I guess. That oneâs up to the developer.
I would expect that rule to flag bad coding practices, and I think it does ;)
Thierry