[prev in list] [next in list] [prev in thread] [next in thread] 

List:       nedit-develop
Subject:    Re: cast to (void)
From:       Thorsten Haude <yoo () vranx ! de>
Date:       2006-10-18 8:43:48
Message-ID: 20061018084348.GC1939 () eumel ! yoo ! local
[Download RAW message or body]

[Attachment #2 (multipart/signed)]


Hi,

* Tony Balinski wrote (2006-10-17 20:01):
>Quoting Thorsten Haude <yoo@vranx.de>:
>> * Tony Balinski wrote (2006-10-17 13:33):
>> >Well, that makes sense. But there's nothing to stop you from doing
>> >so with ones that do.
>>
>> Do what, returning void? Of course, but that would be a buglet. I
>> don't see the point, you can do all kind of things that would worsen
>> the code base.
>No, casting to void the "result" of a void function.

That's what I meant.


>My point is that thedocumentary effect of adding void at the call
>site is nil (the reader can see no returned value is being used)

(I think I mentioned that before.) Yes, but the reader cannot see
whether the returned value is *supposed* to being used.


>the compiler says nothing about faulty coding (rightly, there is no
>faulty code) whether the cast is there or not.

Splint does.


>> Yes, I should have cleared that up: I only suggest this rule for
>> NEdit's own functions, not for libraray calls. The reason is simple:
>> If a coder sees that the return value is not necessary, he can change
>> the function instead of voiding all calls.
>
>I see. Our functions, which aren't necessarily well known, will have their
>calls decorated with "(void)" when no return value is used.

Yes, because that should never happen. Any call which disregards the
return value is an instant reason for refactoring the function or
(more likely) another good thinking about the caller code.)

The only exception are prototype functions.


>> With the changes to the replace truncation I also changed
>> ReplaceInSelection() from returning int to void. Not because I wanted
>> to supress a value, but because the only caller calls it in a void
>> context since version 1.1.
>This is laudable.

But like this other bug which was clearly caused by a missing block
for an if, I would rather remove the source than fix instances.


>> If void casts were used, someone surely
>> would noticed that earlier. This made the code unnecessarily muddy.
>So you would want to "uglify" call sites to force reconsideration of the
>called function's definition. Again, I don't see the point: either the
>function should be corrected (as you did with ReplaceInSelection()), or
>it can validly be called in a way where its return value can be safely
>ignored. If the second, why add the cast?

You know how stuff works, not everyone checks the declaration of every
function he uses trying to find improvements. To make a statistic up
on the spot, probably 10% of NEdit's function parameters are unused
(and I'm not talking about prototype functions here), just because
someone forgot to remove them. (Splint can help here again.)

Also, it might help cohesion if functions either return something
useful or have an effect, never both. The obvious exception are
Boolean values for error reporting on effect functions, but even here
the caller should make the explicit decision to discard this value.
And he should of course document this decision in the code.


>> >I have similar feelings about functions taking no prototypes in C++: by
>> >definition the prototype "int func()" is identical to "int func(void)",
>> >the second form being allowed ONLY for C compatibility, where there IS
>> >a difference (albeit one that provides protection against deprecated
>> >usage).
>>
>> Then I'm glad that we don't have to discuss C++ here. Of course
>> prototypes should explicitly state what arguments, if any, they have.
>With ANSC C prototypes, since 1989, a parameter list of "()" has meant
>"no parameters"; it means "(...)" only with deprecated constructs, which
>C++ doesn't allow. In C++ the argument list "()" has the same meaning as
>"(void)", but is more succinct and, I believe, much clearer.

Succinct aside, I never liked Perl's approach to evaluate "0" and ""
as false, Ruby is much nicer here. If I want something, I should state
it, not leave it open to interpretation to the reader. That is true
even if the case is technically very well defined. I cannot assume
that the next coder has the same level of knowledge that I have, esp.
with Free Software. (I know I have to ask about basics all the time.)


>Seeing
>  sin(pi / 4);
>is just as surprising to me as
>  (void)sin(pi / 4);
>It's just that my eye needs to dig deeper to see that the second makes
>no sense.

Maybe, but you are missing some of the information here, namely that
someone decided to call sin() for its side effects.


>(In the same vein, I don't like the construct "} else {", since it
>distracts from the else-ness of the clause; but I'm really quite
>willing to live with that since it has the advantage of reducing 
>code lines considerably. But that's an old discussion.)

Just to warm that up a bit. As you might remember, my preference is
    if (cond)
    {
        block;
    } else
    {
        block;
    }

Just because it is much easier to see at a glance how the code is made
up if the block is longer than a few lines. Hiding the opening curly
at the end of a line obfuscates that becuse opening and closing curly
are no longer on the same column. The else is easy to see just because
curlies are usually not followed by anything.

I still use this style while coding and have to remember removing it
before commiting.


>> (Good discussion.)
>I'm glad you think so.

Well, that's what we are supposed to do, discussion the finer points
of NEdit's code. We don't have to decide whether the the text should
be rendered bottom-up or whether NEdit should be modal.


>I think adding const is much more than providing usage documentation;
>it limits usage through the compiler in a controlled way.

It does more than that, or you would have to use 'const char* const'
in at least some cases.


>To conclude, if I see (void) in the code, I personally won't be taking it
>out unless I have reason to be changing it.

The latter case is completely valid of course.


>Your suggested usage pattern doesn't sound too onerous either; if you add
>(void)s I will not object (any more ;-)).

I'm not sure I will need another thing to watch our for. It would
really help if someone else would add a voice.


Thorsten                                        Faith Kleppinger: The Hills
-- 
Dieser Satz kein Verb.

[Attachment #5 (application/pgp-signature)]

-- 
NEdit Develop mailing list - Develop@nedit.org
http://www.nedit.org/mailman/listinfo/develop


[prev in list] [next in list] [prev in thread] [next in thread]