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