Hidden argument kind antipattern

[spir]

On 04/20/2011 01:42 AM, Kai Meyer wrote:
> On 04/19/2011 05:18 PM, dsimcha wrote:
>> == Quote from Vladimir Panteleev (vladimir at thecybershadow.net)'s article
>>> To elaborate, I mean allowing code which appears to behave surprisingly
>>> different from the at-a-glance interpretation, unless the programmer knows
>>> the function's signature. I've noticed a worrying adoption in D of this
>>> "antipattern", which, frankly, I believe doesn't belong in a well-designed
>>> programming language. One classic example of this is passing arguments by
>>> reference, something D inherited from C++. For example:
>>> int x, y;
>>> // ...
>>> someObject.updateCoords(x, y);
>>> What can you say about this code? The unobvious and surprising
>>> interpretation of it is that updateCoords will change the values of x and
>>> y. C# solved this problem neatly by requiring to specify the "ref" keyword
>>> before the function arguments:
>>> someObject.updateCoords(ref x, ref y); // much clearer
>>> This problem carries over to lazy parameters, as well. I'll quote a line
>>> of code from a recent post by David Simcha:
>>>> auto connections = taskPool.workerLocalStorage(new MysqlConnection());
>>> Anyone who is not familiar with std.parallelism and D's lazy parameter
>>> feature is going to be very surprised to find out what this code really
>>> does. This might be OK for us, experienced D users, but think about the
>>> poor bloke who will someday, somewhere try to debug a D program written by
>>> someone else, and tear his hair out trying to figure out why an expression
>>> passed as an argument to some function isn't being evaluated before/during
>>> the function call. (The solution I would suggest is simply having to
>>> specify the "lazy" keyword before the expression of each lazy parameter,
>>> same as C#'s "ref". This will require updating all usage of "enforce"
>>> among other changes.)
>>> I know that I should have brought this up during the module's review
>>> phase, so take the following with a grain of salt: in my opinion,
>>> std.parallelism's usage of lazy parameters overreaches their intended use
>>> and borders into abuse of this language feature. For this reason (and
>>> meaning no disrespect towards David and everyone who participated in
>>> shaping the module), I am uncomfortable with the inclusion of
>>> std.parallelism into the standard D library, as it would carry the message
>>> that D's maintainers encourage abusing the language in such ways. (This
>>> probably doesn't count as a vote.) For the moment, I would suggest
>>> changing all lazy parameters which are evaluated in different contexts
>>> (outside of the said function) into delegates, and replacing their usage
>>> with delegate literals.
>>
>> Interesting point. I was questioning whether this was really a good idea myself.
>> I did it because it seemed like things like this were kind of the point of lazy
>> parameters. Let's have a good discussion on this now, and if we decide that my
>> idiom is an abuse of lazy parameters, then I'll switch it over to a delegate
>> literal. My vote is weakly in favor of lazy parameters, since a maintenance
>> programmer would probably look at what such a function does and not be surprised
>> for very long. However, I'm not dead-set against changing it if there's a strong
>> consensus that it should be changed.
>
> I would agree with the "ref" and "lazy" modifiers to parameters. If they exist
> in the function signature, it seems potentially confusing to "silently" accept
> a built-in or other stack-based parameter as a reference with out an explicit
> "ref" in the call. For instance:
>
> import std.stdio;
> struct Bar { public int a; }
> class Foo { public int a; }
> void main()
> {
> int a;
> Bar b = Bar();
> Foo f = new Foo();
> a = 1;
> b.a = 1;
> f.a = 1;
> writef("a=%d b.a=%d f.a=%d\n", a, b.a, f.a);
> changeit(a);
> changeit(b);
> changeit(f);
> writef("a=%d b.a=%d f.a=%d\n", a, b.a, f.a);
> changeit_ref(a);
> changeit_ref(b);
> changeit_ref(f);
> writef("a=%d b.a=%d f.a=%d\n", a, b.a, f.a);
> }
> void changeit(int baz) { baz = 2; }
> void changeit(Bar bar) { bar.a = 2; }
> void changeit(Foo foo) { foo.a = 2; }
> void changeit_ref(ref int baz) { baz = 3; }
> void changeit_ref(ref Bar bar) { bar.a = 3; }
> void changeit_ref(Foo foo) { foo.a = 3; }
> //Output:
> //a=1 b.a=1 f.a=1
> //a=1 b.a=1 f.a=2
> //a=3 b.a=3 f.a=3
>
>
> The call signatures for changeit and changeit_ref are exactly the same, but the
> function definition signatures are different. In the example above, I can see
> the value in forcing the caller to use "changeit_ref(ref a)" to match the
> function declaration.

That's exactly my point of view. It looks stupid & redondant at first sight, 
but its value lies in understanding what code *actually* does, and how. Another 
example, from real code:

     private static Code[] codesFromUTF8 (string s) {
         Code[] codes;
         uint iChar = 0;     // must be uint for UTFDecode
         while (iChar < s.length) {
             // Note: iChar is taken by ref and advanced
             codes ~= std.utf.decode(s, iChar);
         }
         return codes;
     }

I guess without the note it's nearly imossible to understand how this may well 
work. Magic?

Denis
-- 
_________________
vita es estrany
spir.wikidot.com