[fpc-devel] Suggestion for change: Overly strict check

[Christian Iversen]

On Monday 02 October 2006 11:46, Ales Katona wrote:
> Having same function names as parameter names per se isn't a biggy but
> the biggest problem is:
>
> TFirst = class
>  protected
>   FFirst: Integer;
>  public
>   property First: Integer read FFirst write FFirst;
> end;
>
> TTest = class(TFirst)
>   FSomething: Integer;
>  public
>   procedure DoWithFirst(a, First: Integer);
> end;
>
> implementation
>
> procedure TTest.DoWithFirst(a, First: Integer);
> begin
>   First:=a; // ???
>   FSomething:=First; // ???
> end;

Ales, there is _no_ confusion here. Normal scope rules ensures that the first 
line only assigns the param "a" to the param "first". There are no other ways 
to interpret that.

The second line assigns the param "First" to the private field "FSomething". 
Again, no problem at all.

> Ofcourse the intention was setting FSomething to value of argument First
> and setting FFirst to value of First. It CAN be achieved with "FFirst"
> or "self.First" but it's just a stupid example of how easily you can
> overlook things especialy if some parts are hidden in another unit
> somewhere in an ancestor.

This is very much less of a problem in practice, I believe.

Really, perhaps what we would like to check, is that there are no fields of 
the same name, _with_a_compatible_type_? That would give warnings that are 
much better, and still allow (non-method-pointer) parameters in one method to 
be named the same as another method. 

Wouldn't that make more sense?

> Also notice that in mode delphi there's not a single warning about this.

True - because it's OK to do this. There are no warnings about all sorts of 
other normal behaviour either :)

> And believe me things like this CAN happen and will take you 3 days to
> find out...
>
> I name my arguments "aName" since this incident and not because
> {$objfpc}...

Ok.

> PS: there are worse cases but I can't remember a better example now

Let me know if you remember them :)

-- 
Regards,
Christian Iversen