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 <https://pharo.fogbugz.com/default.asp?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