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
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