www.digitalmars.com         C & C++   DMDScript  

digitalmars.D - try/catch idiom in std.datetime

reply Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
I'm of a stance that good code should have many throws and few try/catch 
statements. I was curious how Phobos does there, so I ran a few simple 
greps around.

Seems like there's about 145 try statements in Phobos. Of these, more 
than a third (51) belong to std.datetime.

Looks like the following idiom is often present in std.datetime:

      property FracSec fracSec() const nothrow
     {
         try
         {
             auto hnsecs = removeUnitsFromHNSecs!"days"(adjTime);

             if(hnsecs < 0)
                 hnsecs += convert!("hours", "hnsecs")(24);

             hnsecs = removeUnitsFromHNSecs!"seconds"(hnsecs);

             return FracSec.from!"hnsecs"(cast(int)hnsecs);
         }
         catch(Exception e)
             assert(0, "FracSec.from!\"hnsecs\"() threw.");
     }

(It may seem I chose this example to discuss the "let's insert one empty 
line every other line" idiom. I didn't.) Essentially the code relies on 
calls that may generally throw, but calls them with parameters that 
should guarantee there won't be any throwing. In wanting to offer as 
many "nothrow" guarantees as possible, the code ends up inserting these 
try/catch statements - seemingly needlessly.

This is quite heavy-handed, so I was wondering what we could do to 
improve on this. I thought of the following possibility:

      property FracSec fracSec() const nothrow
     {
         scope(failure) assert(0, "FracSec.from!\"hnsecs\"() threw.");
         auto hnsecs = removeUnitsFromHNSecs!"days"(adjTime);
         if (hnsecs < 0)
             hnsecs += convert!("hours", "hnsecs")(24);
         hnsecs = removeUnitsFromHNSecs!"seconds"(hnsecs);
         return FracSec.from!"hnsecs"(cast(int)hnsecs);
     }

This at least leaves the normal path unaltered and deals with the 
unexpected in a more isolated manner. I was pleased that the changed 
code compiled just fine. My happiness was short-lived because before 
long I figured this compiles as well:

         ...
         scope(failure) {}
         ...

So it's not that the compiler cleverly figured the function will not 
throw. Instead, the compiler considers everything dandy as soon as it 
sees a scope(failure), no matter of the statement it controls. I'll 
report that bug soon.

What would be the best approach here?

0. Do nothing. This is as good as it gets.

1. Fix scope(failure) and then use it.

2. Relax the nothrow guarantees. After all, nothrow is opt-in.

3. Look into API changes that add nothrow, narrower functions to the 
more general ones that may throw.

4. ...?


Andrei
Nov 17 2013
next sibling parent reply Jonathan M Davis <jmdavisProg gmx.com> writes:
On Sunday, November 17, 2013 22:32:46 Andrei Alexandrescu wrote:

 (It may seem I chose this example to discuss the "let's insert one empty
 line every other line" idiom. I didn't.)


That code's like that just because I like to put empty lines before and after 
if statements and before return statements, as I think that that improves 
legibility. Short functions like that suffer as a result, because they end up 
with a larger proportion of the lines being empty than is normal. I'm always a 
bit torn on that, because I don't like having quite that many empty lines in 
so few lines, but I also don't like not having space around if statements and 
return statements. I never feel like I can find a nice, legible balance with 
short functions like that.


 Essentially the code relies on
 calls that may generally throw, but calls them with parameters that
 should guarantee there won't be any throwing. In wanting to offer as
 many "nothrow" guarantees as possible, the code ends up inserting these
 try/catch statements - seemingly needlessly.


Yeah, I tried to use pure and nothrow fairly heavily in std.datetime and ran 
into a number of hurdles like this. Fortunately, the situation has improved 
somewhat (e.g. format can finally be pure at least some of the time), but it 
does show that it's not always easy to use pure or nothrow even when it 
arguably should be.


 What would be the best approach here?
 
 0. Do nothing. This is as good as it gets.


Well, it works just fine as-is, but it would be kind of nice to be able to 
solve the problem in a less verbose manner (though you're talking about saving 
only a few lines of code).


 1. Fix scope(failure) and then use it.


Fine with me. I might still favor the try-catch in cases where you can clearly 
wrap it around one function call, because then you avoid problems where you've 
accidentally effectively marked the whole function as trusted-nothrow when you 
only want to mark one function call that way. But you could do the same thing 
with scope(failure) and a new scope. The main problem is when you can't really 
put the calls that need to be trusted-nothrow inside a new scope, in which 
case, you're forced to mark the whole function (or at least large portions of 
it) as trusted-nothrow by wrapping it all in a try-catch or scope(failure).


 2. Relax the nothrow guarantees. After all, nothrow is opt-in.


I'm not quite sure what you're suggesting here. Make it so that nothrow does 
checking at runtime instead of compile time? That would be moving in the 
direction of C++ and throw specifiers (or more precisely, noexcept, I suppose). 
If that's what you're suggesting, I'd be very much against that. I think that 
the fact D's nothrow is statically checked is a huge advantage over C++'s 
noexcept. The fact that you have to sometimes use try-catch blocks (or 
scope(failure) if that works) to make it work is essentially the same as 
needing  trusted to make some stuff  safe. I wouldn't want to throw away 
 trusted in favor of making  safe more lax either (though that's almost all 
static checking which can't be done at runtime, unlike with noexcept).

Of course, there's no way to verify trusted-nothrow except at runtime like 
std.datetime is doing with try-catch and assertions, but most code _can_ be 
checked statically (including the code that calls the functions that use the 
try-catch-assert idiom to be able to be nothrow), and that's much more 
pleasant, particularly because it's actually checked by the compiler that way 
rather than just blowing up on you at runtime.

I suppose that if it were considered annoying enough to have to use try-catch 
blocks or scope(failure), we could add a nothrow equivalent to  trusted to 
mark functions with, though it's already been argued that  trusted should be 
on pieces of a function rather than on the whole function, and it would 
arguably be better to mark sections of a function as trusted-nothrow rather 
than the entire thing. try-catch lets us do that already, but it might be nice 
to be able to do the equivalent of

      property FracSec fracSec() const nothrow
     {
         trusted-nothrow
         {
             auto hnsecs = removeUnitsFromHNSecs!"days"(adjTime);

             if(hnsecs < 0)
                 hnsecs += convert!("hours", "hnsecs")(24);

             hnsecs = removeUnitsFromHNSecs!"seconds"(hnsecs);

             return FracSec.from!"hnsecs"(cast(int)hnsecs);
         }
     }

and have the compiler insert the catch and assertion for you.


 3. Look into API changes that add nothrow, narrower functions to the
 more general ones that may throw.


In some cases (e.g. format) that's probably a good idea, but I don't think 
that really scales. In some cases, it would be well worth it, whereas in 
others, it would just be better to use try-catch in the few places where you 
know the function won't throw, because it would be quite rare to be able to 
guarantee that it wouldn't throw. It's also not pleasant to have to duplicate 
functions all over the place.


 4. ...?


We now have std.exception.assumeWontThrow, which works reasonably well when 
you need to wrap a single call as opposed to several, but it has the same 
problem as enforce in that it uses lazy, which is definite performance hit. So, 
in most cases, I'd be more inclined to just use a try-catch, and if it's more 
than one expression, you pretty much need to use try-catch or scope(failure) 
instead anyway, since you wouldn't want to wrap whole function bodies in a 
call to assumeWontThrow (assuming that you even could).

So, that's a partial solution, but not a perfomant one. However, we do really 
need to improve the performance of lazy, because enforce gets used all over 
the place, and it's definitely shown up as being costly in some of the
profiling 
that's been shown in the newsgroup. And if that gets fixed, then using 
assumeWontThrow wouldn't be as bad.

All in all, I find the need to use try-catch blocks to effectively do trusted-
nothrow a bit annoying, but I haven't felt that it was a big enough deal to 
try and find another solution for it. Having to do trusted-purity is far worse, 
because that requires using a function pointer and casting it, but that would 
_should_ be hard to do because of how hard it is to actually guarantee that 
the function is acting like a pure function even though it isn't. And I'm not 
sure that I'd entirely trust myself with that, let alone the average D 
developer. trusted-nothrow on the other hand is something that the average 
programmer should be able to grasp.

- Jonathan M Davis
Nov 17 2013
next sibling parent reply Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
On 11/17/13 11:28 PM, Jonathan M Davis wrote:

 On Sunday, November 17, 2013 22:32:46 Andrei Alexandrescu wrote:

 (It may seem I chose this example to discuss the "let's insert one empty
 line every other line" idiom. I didn't.)


 That code's like that just because I like to put empty lines before and after
 if statements and before return statements, as I think that that improves
 legibility. Short functions like that suffer as a result, because they end up
 with a larger proportion of the lines being empty than is normal. I'm always a
 bit torn on that, because I don't like having quite that many empty lines in
 so few lines, but I also don't like not having space around if statements and
 return statements. I never feel like I can find a nice, legible balance with
 short functions like that.


"Too much of a good thing" comes to mind (as does with the massive and 
repetitive unittests in std.datetime). If code looks and feels meh, it 
probably is. Just apply good judgment instead of rote adherence to rules.


 0. Do nothing. This is as good as it gets.


 Well, it works just fine as-is, but it would be kind of nice to be able to
 solve the problem in a less verbose manner (though you're talking about saving
 only a few lines of code).


I'm also concerned about generated code size and overall efficiency. It 
looks like the assert(0) insertions are there simply to validate the 
design (they only fail if Phobos has an internal error), so there should 
be some means to remove them in release builds. We don't have such a 
possibility at the moment.


 2. Relax the nothrow guarantees. After all, nothrow is opt-in.


 I'm not quite sure what you're suggesting here.


What I meant is, not everything that is nothrow needs to be annotated as 
such.


        property FracSec fracSec() const nothrow
       {
           trusted-nothrow
           {
               auto hnsecs = removeUnitsFromHNSecs!"days"(adjTime);

               if(hnsecs < 0)
                   hnsecs += convert!("hours", "hnsecs")(24);

               hnsecs = removeUnitsFromHNSecs!"seconds"(hnsecs);

               return FracSec.from!"hnsecs"(cast(int)hnsecs);
           }
       }


That can be done at library level.


 4. ...?


 We now have std.exception.assumeWontThrow, which works reasonably well when
 you need to wrap a single call as opposed to several, but it has the same
 problem as enforce in that it uses lazy, which is definite performance hit. So,
 in most cases, I'd be more inclined to just use a try-catch, and if it's more
 than one expression, you pretty much need to use try-catch or scope(failure)
 instead anyway, since you wouldn't want to wrap whole function bodies in a
 call to assumeWontThrow (assuming that you even could).


Again, one problem here (in addition to the blowup in code size and 
decay of readability) is we're talking about an internal design 
validation, not meaningful runtime semantics. There should be a way to 
remove the baggage.


Andrei
Nov 18 2013
next sibling parent Jonathan M Davis <jmdavisProg gmx.com> writes:
On Monday, November 18, 2013 00:14:24 Andrei Alexandrescu wrote:

 On 11/17/13 11:28 PM, Jonathan M Davis wrote:

 On Sunday, November 17, 2013 22:32:46 Andrei Alexandrescu wrote:


 "Too much of a good thing" comes to mind (as does with the massive and
 repetitive unittests in std.datetime). If code looks and feels meh, it
 probably is. Just apply good judgment instead of rote adherence to rules.


Well, my coding style has adjusted over time, and I'm tending to put fewer 
blank lines in cases like this, precisely because it's not terribly pretty.


 as does with the massive and repetitive unittests in std.datetime


Those are quite valuable IMHO, as being thorough has caught a lot of problems, 
and as we've discussed (and disagreed on) before, I prefer unit tests to be 
dead simple in order to reduce the odds of them being buggy, even if that 
means that they're more verbose. I have done some work however to move the 
tests toward using loops instead in order to reduce the number of lines of 
code, but I have quite a bit still to do there. I'll probably get back to that 
after I finish with splitting std.datetime, but I haven't finished that yet 
because of how busy I've been the past few months. So, everything takes 
forever... Oh well, I should get back to that soon enough.


 0. Do nothing. This is as good as it gets.


 
 Well, it works just fine as-is, but it would be kind of nice to be able to
 solve the problem in a less verbose manner (though you're talking about
 saving only a few lines of code).


 
 I'm also concerned about generated code size and overall efficiency. It
 looks like the assert(0) insertions are there simply to validate the
 design (they only fail if Phobos has an internal error), so there should
 be some means to remove them in release builds. We don't have such a
 possibility at the moment.


Well, the assertions are there to validate the design, but the try-catches are 
required to make the function nothrow, and I'd feel very funny having an empty 
catch block, though having it compiled out in -release wouldn't necessarily be 
a bad idea (which could probably be done if a function which returned false 
where used in the assertion instead of a constant).


 2. Relax the nothrow guarantees. After all, nothrow is opt-in.


 
 I'm not quite sure what you're suggesting here.


 
 What I meant is, not everything that is nothrow needs to be annotated as
 such.


So, you're suggesting that nothrow be inferred?


        property FracSec fracSec() const nothrow
       {
       
           trusted-nothrow
           {
           
               auto hnsecs = removeUnitsFromHNSecs!"days"(adjTime);
               
               if(hnsecs < 0)
               
                   hnsecs += convert!("hours", "hnsecs")(24);
               
               hnsecs = removeUnitsFromHNSecs!"seconds"(hnsecs);
               
               return FracSec.from!"hnsecs"(cast(int)hnsecs);
           
           }
       
       }


 
 That can be done at library level.


Well, it is being done at the library level right now via try-catch.


 4. ...?


 
 We now have std.exception.assumeWontThrow, which works reasonably well
 when
 you need to wrap a single call as opposed to several, but it has the same
 problem as enforce in that it uses lazy, which is definite performance
 hit. So, in most cases, I'd be more inclined to just use a try-catch, and
 if it's more than one expression, you pretty much need to use try-catch
 or scope(failure) instead anyway, since you wouldn't want to wrap whole
 function bodies in a call to assumeWontThrow (assuming that you even
 could).


 
 Again, one problem here (in addition to the blowup in code size and
 decay of readability) is we're talking about an internal design
 validation, not meaningful runtime semantics. There should be a way to
 remove the baggage.


I think that that would require a language change of some kind, because it 
would require that the compiler recognize that the check can go away in
-release, which would effectively mean having the try-catch block be optimized 
away somehow when compiling with -release. Using scope(failure) for that 
wouldn't change anything, as it would just mean that the compiler was 
inserting the try-catch statements for you. It would just be slightly less 
verbose.

Maybe the compiler can be changed to optimize out try-catch blocks if the 
catch block just contains an assertion? That would at least get rid of the 
overhead of the idiom, even if it didn't make it less verbose. If we can't get 
the compiler to recognize the idiom and optimize out the unnecessary code, 
then if we want it to be optimized out, I think that we'll be forced to add 
something to the language which the compiler _does_ recognize and therefore 
can compile out when appropriate - unless someone can come up with some great 
idea for improving nothrow, but that seems a lot like it would be moving 
towards not needing  trusted anymore, because the compiler would figure that 
out for you. I don't see how you could get away from having to have the 
programmer tell the compiler that they're sure that the function which can 
throw won't throw inside the nothrow function, because that requires human 
judgement just like  trusted does, though it should generally be easier for a 
programmer to be sure that something won't throw than it is to be sure that 
it's actually  safe.

- Jonathan M Davis
Nov 18 2013
prev sibling parent reply Jacob Carlborg <doob me.com> writes:
On 2013-11-18 09:14, Andrei Alexandrescu wrote:


 I'm also concerned about generated code size and overall efficiency. It
 looks like the assert(0) insertions are there simply to validate the
 design (they only fail if Phobos has an internal error), so there should
 be some means to remove them in release builds. We don't have such a
 possibility at the moment.


Asserts are supposed to be removed in release builds. There's also the 
"debug" statement that can be used. Although it won't remove the try-catch.

-- 
/Jacob Carlborg
Nov 18 2013
parent reply Jonathan M Davis <jmdavisProg gmx.com> writes:
On Monday, November 18, 2013 10:09:35 Jacob Carlborg wrote:

 On 2013-11-18 09:14, Andrei Alexandrescu wrote:

 I'm also concerned about generated code size and overall efficiency. It
 looks like the assert(0) insertions are there simply to validate the
 design (they only fail if Phobos has an internal error), so there should
 be some means to remove them in release builds. We don't have such a
 possibility at the moment.


 
 Asserts are supposed to be removed in release builds.


The aren't if the condition is statically known to be false, as is the case 
when you use a try-catch block to catch Exception and then assert within the 
catch block in order to make the function be able to be nothrow.


 There's also the
 "debug" statement that can be used. Although it won't remove the try-catch.


It also won't run unless you compile with -debug, which I expect very, very 
few people do.

- Jonathan M Davis
Nov 18 2013
parent Jacob Carlborg <doob me.com> writes:
On 2013-11-18 10:13, Jonathan M Davis wrote:


 The aren't if the condition is statically known to be false, as is the case
 when you use a try-catch block to catch Exception and then assert within the
 catch block in order to make the function be able to be nothrow.


Then it supposed to turn it into a HLT instruction, at least on x86. I 
don't know how much that affects the code size.


 It also won't run unless you compile with -debug, which I expect very, very
 few people do.


True.

-- 
/Jacob Carlborg
Nov 18 2013
prev sibling next sibling parent reply Jacob Carlborg <doob me.com> writes:
On 2013-11-18 08:28, Jonathan M Davis wrote:


 That code's like that just because I like to put empty lines before and after
 if statements and before return statements, as I think that that improves
 legibility. Short functions like that suffer as a result, because they end up
 with a larger proportion of the lines being empty than is normal. I'm always a
 bit torn on that, because I don't like having quite that many empty lines in
 so few lines, but I also don't like not having space around if statements and
 return statements. I never feel like I can find a nice, legible balance with
 short functions like that.


I like the empty newlines as well. Otherwise it feels like reading a 
text without paragraphs.


 Fine with me. I might still favor the try-catch in cases where you can clearly
 wrap it around one function call, because then you avoid problems where you've
 accidentally effectively marked the whole function as trusted-nothrow when you
 only want to mark one function call that way. But you could do the same thing
 with scope(failure) and a new scope. The main problem is when you can't really
 put the calls that need to be trusted-nothrow inside a new scope, in which
 case, you're forced to mark the whole function (or at least large portions of
 it) as trusted-nothrow by wrapping it all in a try-catch or scope(failure).


In this case the whole function is wrapped on a try-block. In those 
cases I think a scope(failure) looks nicer.


 2. Relax the nothrow guarantees. After all, nothrow is opt-in.


 I'm not quite sure what you're suggesting here. Make it so that nothrow does
 checking at runtime instead of compile time? That would be moving in the
 direction of C++ and throw specifiers (or more precisely, noexcept, I suppose).
 If that's what you're suggesting, I'd be very much against that. I think that
 the fact D's nothrow is statically checked is a huge advantage over C++'s
 noexcept. The fact that you have to sometimes use try-catch blocks (or
 scope(failure) if that works) to make it work is essentially the same as
 needing  trusted to make some stuff  safe. I wouldn't want to throw away
  trusted in favor of making  safe more lax either (though that's almost all
 static checking which can't be done at runtime, unlike with noexcept).

 Of course, there's no way to verify trusted-nothrow except at runtime like
 std.datetime is doing with try-catch and assertions, but most code _can_ be
 checked statically (including the code that calls the functions that use the
 try-catch-assert idiom to be able to be nothrow), and that's much more
 pleasant, particularly because it's actually checked by the compiler that way
 rather than just blowing up on you at runtime.

 I suppose that if it were considered annoying enough to have to use try-catch
 blocks or scope(failure), we could add a nothrow equivalent to  trusted to
 mark functions with, though it's already been argued that  trusted should be
 on pieces of a function rather than on the whole function, and it would
 arguably be better to mark sections of a function as trusted-nothrow rather
 than the entire thing. try-catch lets us do that already, but it might be nice
 to be able to do the equivalent of

        property FracSec fracSec() const nothrow
       {
           trusted-nothrow
           {
               auto hnsecs = removeUnitsFromHNSecs!"days"(adjTime);

               if(hnsecs < 0)
                   hnsecs += convert!("hours", "hnsecs")(24);

               hnsecs = removeUnitsFromHNSecs!"seconds"(hnsecs);

               return FracSec.from!"hnsecs"(cast(int)hnsecs);
           }
       }

 and have the compiler insert the catch and assertion for you.


Just do something like this:

void trusted_nothrow (alias block) ()
{
     scope(failure) assert(0);
     block();
}

trusted_nothrow!({
     // code
});

Not as pretty. Yet another prefect example for AST macros :)

-- 
/Jacob Carlborg
Nov 18 2013
next sibling parent reply Jonathan M Davis <jmdavisProg gmx.com> writes:
On Monday, November 18, 2013 09:59:44 Jacob Carlborg wrote:

 Just do something like this:
 
 void trusted_nothrow (alias block) ()
 {
      scope(failure) assert(0);
      block();
 }
 
 trusted_nothrow!({
      // code
 });
 
 Not as pretty. Yet another prefect example for AST macros :)


I don't see much gain over simply putting scope(failure) at the top. You just 
avoid having to write the assertion yourself. I also expect that it wouldn't 
work with return statements.

- Jonathan M Davis
Nov 18 2013
parent reply Jacob Carlborg <doob me.com> writes:
On 2013-11-18 10:18, Jonathan M Davis wrote:


 I don't see much gain over simply putting scope(failure) at the top. You just
 avoid having to write the assertion yourself


You suggested it, but by adding it to the language instead.


 I also expect that it wouldn't work with return statements.


No. It would just return from the delegate.

-- 
/Jacob Carlborg
Nov 18 2013
parent Jonathan M Davis <jmdavisProg gmx.com> writes:
On Monday, November 18, 2013 10:24:11 Jacob Carlborg wrote:

 On 2013-11-18 10:18, Jonathan M Davis wrote:

 I don't see much gain over simply putting scope(failure) at the top. You
 just avoid having to write the assertion yourself


 
 You suggested it, but by adding it to the language instead.


True, but if the language did it, it could deal with it in a manner which 
compiled out the try-catch in -release mode as well as avoid the ugliness of 
wrapping the whole function body in a function call. Still, it would probably 
be better if we could get the compiler to recognize the idiom and compile out 
the try-catch in that case rather than adding something like that to the 
language. But I don't know how reasonable that is.

- Jonathan m Davis
Nov 18 2013
prev sibling parent reply Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
On 11/18/13 12:59 AM, Jacob Carlborg wrote:

 On 2013-11-18 08:28, Jonathan M Davis wrote:


 That code's like that just because I like to put empty lines before
 and after
 if statements and before return statements, as I think that that improves
 legibility. Short functions like that suffer as a result, because they
 end up
 with a larger proportion of the lines being empty than is normal. I'm
 always a
 bit torn on that, because I don't like having quite that many empty
 lines in
 so few lines, but I also don't like not having space around if
 statements and
 return statements. I never feel like I can find a nice, legible
 balance with
 short functions like that.


 I like the empty newlines as well. Otherwise it feels like reading a
 text without paragraphs.


But three newlines for four lines? That's not paragraphs, it's a grocery 
list.

Andrei
Nov 18 2013
next sibling parent reply "Jonathan M Davis" <jmdavisProg gmx.com> writes:
On Monday, November 18, 2013 07:56:15 Andrei Alexandrescu wrote:

 On 11/18/13 12:59 AM, Jacob Carlborg wrote:

 I like the empty newlines as well. Otherwise it feels like reading a
 text without paragraphs.


 
 But three newlines for four lines? That's not paragraphs, it's a grocery
 list.


Yeah. It falls apart on some level when there are so few lines. It's just that 
it's also ugly to mush them all together. So, to me at least, it seems like 
it's ugly either way, and in the past, I've just stuck with my normal pattern 
of having the blank lines when that's the case. Now (in part, thanks to your 
complaints about it), I'm more likely to mush the lines together when putting 
the blank lines would result in a bunch of one liners. But I don't really feel 
like there's a good solution when the function is so short.

- Jonathan M Davis
Nov 18 2013
parent reply Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
On 11/18/13 12:30 PM, Jonathan M Davis wrote:

 On Monday, November 18, 2013 07:56:15 Andrei Alexandrescu wrote:

 On 11/18/13 12:59 AM, Jacob Carlborg wrote:

 I like the empty newlines as well. Otherwise it feels like reading a
 text without paragraphs.


 But three newlines for four lines? That's not paragraphs, it's a grocery
 list.


 Yeah. It falls apart on some level when there are so few lines. It's just that
 it's also ugly to mush them all together. So, to me at least, it seems like
 it's ugly either way, and in the past, I've just stuck with my normal pattern
 of having the blank lines when that's the case. Now (in part, thanks to your
 complaints about it), I'm more likely to mush the lines together when putting
 the blank lines would result in a bunch of one liners. But I don't really feel
 like there's a good solution when the function is so short.


Well you must agree at some level that since you don't like short 
functions with or without newlines inserted, the set of options is 
considerably reduced.

1. You search for an editor that offers half lines.

2. You shun short functions altogether.

3. You adjust your style.

Style is not something immutable. Paradoxically, when I was younger I 
used to see it as less of a variable than now. Improving and adapting 
one's style to various contexts is a lifelong and beneficial process.


Andrei
Nov 18 2013
parent "Jonathan M Davis" <jmdavisProg gmx.com> writes:
On Monday, November 18, 2013 12:44:43 Andrei Alexandrescu wrote:

 Well you must agree at some level that since you don't like short
 functions with or without newlines inserted, the set of options is
 considerably reduced.
 
 1. You search for an editor that offers half lines.
 
 2. You shun short functions altogether.
 
 3. You adjust your style.
 
 Style is not something immutable. Paradoxically, when I was younger I
 used to see it as less of a variable than now. Improving and adapting
 one's style to various contexts is a lifelong and beneficial process.


I agree. My style has adjusted over time. It's just that I don't like any of 
the options in this case, so I'm divided on how to format the code. But given 
how others dislike the empty lines, I now lean more towards going with fewer 
lines in this case. At least then other folks will like the formatting more 
even if both ways bug may. But maybe it'll grow on me and bug me less.

- Jonathan M Davis
Nov 18 2013
prev sibling parent reply Jacob Carlborg <doob me.com> writes:
On 2013-11-18 16:56, Andrei Alexandrescu wrote:


 But three newlines for four lines? That's not paragraphs, it's a grocery
 list.


As Jonathan said, this example is quite bad in the empty lines vs code 
lines ratio.

But I general I do put an empty newline before and after each statement.

BTW, I don't have any empty newlines in my grocery list :) . It's more 
of bullet point list. Or I use an app on my phone which handles all that 
for me.

-- 
/Jacob Carlborg
Nov 18 2013
parent reply Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
On 11/18/13 12:53 PM, Jacob Carlborg wrote:

 On 2013-11-18 16:56, Andrei Alexandrescu wrote:


 But three newlines for four lines? That's not paragraphs, it's a grocery
 list.


 As Jonathan said, this example is quite bad in the empty lines vs code
 lines ratio.

 But I general I do put an empty newline before and after each statement.


So that would be two empty lines between statements :o). Anyhow, 
assuming you only meant one, that would be excessive if done for _all_ 
statements (i.e. everything ending with a semicolon). It would make code 
look like a doc printed by someone who didn't know how to turn double 
spacing off.


Andrei
Nov 18 2013
parent reply Jacob Carlborg <doob me.com> writes:
On 2013-11-18 23:33, Andrei Alexandrescu wrote:


 So that would be two empty lines between statements :o). Anyhow,
 assuming you only meant one, that would be excessive if done for _all_
 statements (i.e. everything ending with a semicolon). It would make code
 look like a doc printed by someone who didn't know how to turn double
 spacing off.


I don't agree. And not having any empty newlines in the code would be 
like reading a text without paragraphs, as I've already said.

Instead, take a look at some code I've written and you're free to 
thinking whatever you like, but I like that style.

https://github.com/jacob-carlborg/dstep/blob/master/dstep/translator/Translator.d

-- 
/Jacob Carlborg
Nov 18 2013
next sibling parent reply Jonathan M Davis <jmdavisProg gmx.com> writes:
On Tuesday, November 19, 2013 08:30:56 Jacob Carlborg wrote:

 On 2013-11-18 23:33, Andrei Alexandrescu wrote:

 So that would be two empty lines between statements :o). Anyhow,
 assuming you only meant one, that would be excessive if done for _all_
 statements (i.e. everything ending with a semicolon). It would make code
 look like a doc printed by someone who didn't know how to turn double
 spacing off.


 
 I don't agree. And not having any empty newlines in the code would be
 like reading a text without paragraphs, as I've already said.
 
 Instead, take a look at some code I've written and you're free to
 thinking whatever you like, but I like that style.
 
 https://github.com/jacob-carlborg/dstep/blob/master/dstep/translator/Transla
 tor.d


That looks pretty good overall, though there are probably a few places where 
I'd favor having fewer newlines. e.g. the body of the foreach in 
translateFunction is formatted similarly to what I'd do by default, but the 
overall feel is that it has too much vertical space. So, I might be inclined 
to remove some of the newlines, much as I'd want them by default. The fact 
that you have braces on the first version block doesn't help either. But the 
formatting is okay.

While I suspect that Andrei would definitely want to remove many of those 
newlines if he were formatting the code, what I think Andrei was really 
objecting to in your comments said that you wanted to put a newline after 
_every_ statement (which you're not actually doing in your code), and then you 
wanted separate lines between "paragraphs of code," implying that you'd end up 
with two newlines separating out sections of your functions. And that would be 
a lot of extraneous newlines. What you actually seem to have is putting a 
newline around "paragraphs" but not a newline between every statement, which 
is a lot more reasonable (though if your "paragraphs" are too small, you run 
into the exact problem that Andrei was commenting on in my code that started 
this discussion).

By the way, you have a lot of useless breaks in your code. A break isn't 
actually required if there's a return statement or other control statement 
such as continue at the end of the case statement. Maybe it's a stylistic 
thing? But in case you didn't know, they're completely unnecessary, and 
personally, I'd consider them to be clutter. It's your code though, not mine.

- Jonathan M Davis
Nov 19 2013
parent Jacob Carlborg <doob me.com> writes:
On 2013-11-19 11:32, Jonathan M Davis wrote:


 That looks pretty good overall, though there are probably a few places where
 I'd favor having fewer newlines. e.g. the body of the foreach in
 translateFunction is formatted similarly to what I'd do by default, but the
 overall feel is that it has too much vertical space. So, I might be inclined
 to remove some of the newlines, much as I'd want them by default. The fact
 that you have braces on the first version block doesn't help either. But the
 formatting is okay.


That particular version block is from a pull request and doesn't follow 
the overall formatting. I wasn't so picky about the formatting in this case.


 While I suspect that Andrei would definitely want to remove many of those
 newlines if he were formatting the code, what I think Andrei was really
 objecting to in your comments said that you wanted to put a newline after
 _every_ statement (which you're not actually doing in your code), and then you
 wanted separate lines between "paragraphs of code," implying that you'd end up
 with two newlines separating out sections of your functions. And that would be
 a lot of extraneous newlines. What you actually seem to have is putting a
 newline around "paragraphs" but not a newline between every statement, which
 is a lot more reasonable (though if your "paragraphs" are too small, you run
 into the exact problem that Andrei was commenting on in my code that started
 this discussion).


Looking at code in Phobos I would suspect Andrei wanting to remove all 
empty newlines inside functions.

No, I don't want two empty newlines next to each other. I guess I didn't 
phrase my self very clearly. Your explanation of putting a newline 
around "paragraphs" seems better.


 By the way, you have a lot of useless breaks in your code. A break isn't
 actually required if there's a return statement or other control statement
 such as continue at the end of the case statement. Maybe it's a stylistic
 thing? But in case you didn't know, they're completely unnecessary, and
 personally, I'd consider them to be clutter. It's your code though, not mine.


Hmm, I don't remember how I ended up with that. I do know that they're 
not required. Perhaps it was a stylistic choice. I think it groups 
things nicely, just as braces do.

-- 
/Jacob Carlborg
Nov 19 2013
prev sibling parent reply Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
On 11/18/13 11:30 PM, Jacob Carlborg wrote:

 On 2013-11-18 23:33, Andrei Alexandrescu wrote:


 So that would be two empty lines between statements :o). Anyhow,
 assuming you only meant one, that would be excessive if done for _all_
 statements (i.e. everything ending with a semicolon). It would make code
 look like a doc printed by someone who didn't know how to turn double
 spacing off.


 I don't agree. And not having any empty newlines in the code would be
 like reading a text without paragraphs, as I've already said.

 Instead, take a look at some code I've written and you're free to
 thinking whatever you like, but I like that style.

 https://github.com/jacob-carlborg/dstep/blob/master/dstep/translator/Translator.d


That doesn't have an empty line after each statement.

Andrei
Nov 19 2013
parent reply Jacob Carlborg <doob me.com> writes:
On 2013-11-19 16:01, Andrei Alexandrescu wrote:


 That doesn't have an empty line after each statement.


If an expression or another statement follows, then yes. But not if a 
closing brace follows. I guess I didn't phrase myself very clearly.

-- 
/Jacob Carlborg
Nov 19 2013
parent reply Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
On 11/19/13 7:45 AM, Jacob Carlborg wrote:

 On 2013-11-19 16:01, Andrei Alexandrescu wrote:


 That doesn't have an empty line after each statement.


 If an expression or another statement follows, then yes.


You're not operating with the correct definitions of "expression" and 
"statement".

Andrei
Nov 19 2013
parent reply Jacob Carlborg <doob me.com> writes:
On 2013-11-19 16:56, Andrei Alexandrescu wrote:


 You're not operating with the correct definitions of "expression" and
 "statement".


I would consider this a statement: 
https://github.com/jacob-carlborg/dstep/blob/master/dstep/translator/Translator.d#L285..L286

And this an expression: 
https://github.com/jacob-carlborg/dstep/blob/master/dstep/translator/Translator.d#L288

The first would be an if-statement [1] and the second would be an assign 
expression [2]. Am I wrong?

[1] http://dlang.org/statement.html#IfStatement
[2] http://dlang.org/expression.html#AssignExpression

-- 
/Jacob Carlborg
Nov 19 2013
next sibling parent reply "David Nadlinger" <code klickverbot.at> writes:
On Tuesday, 19 November 2013 at 16:07:17 UTC, Jacob Carlborg 
wrote:

 On 2013-11-19 16:56, Andrei Alexandrescu wrote:


 You're not operating with the correct definitions of 
 "expression" and
 "statement".


 I would consider this a statement: 
 https://github.com/jacob-carlborg/dstep/blob/master/dstep/translator/Translator.d#L285..L286

 And this an expression: 
 https://github.com/jacob-carlborg/dstep/blob/master/dstep/translator/Translator.d#L288


http://dlang.org/statement.html#ExpressionStatement

David
Nov 19 2013
parent Jacob Carlborg <doob me.com> writes:
On 2013-11-19 18:59, David Nadlinger wrote:


 http://dlang.org/statement.html#ExpressionStatement


Ok, I didn't know that.

-- 
/Jacob Carlborg
Nov 19 2013
prev sibling parent reply Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
On 11/19/13 8:07 AM, Jacob Carlborg wrote:

 On 2013-11-19 16:56, Andrei Alexandrescu wrote:


 You're not operating with the correct definitions of "expression" and
 "statement".


 I would consider this a statement:
 https://github.com/jacob-carlborg/dstep/blob/master/dstep/translator/Translator.d#L285..L286


Sure is.


 And this an expression:
 https://github.com/jacob-carlborg/dstep/blob/master/dstep/translator/Translator.d#L288


That's not an expression, it's a statement - more precisely an 
expression statement. (The semicolon makes it so.) By the rules you 
incorrectly stated, there should be an empty line after it.

I'll allow myself a piece of advice - the density of e.g. 
https://github.com/jacob-carlborg/dstep/blob/master/dstep/translat
r/Translator.d#L251 
is low enough to make it career limiting. You'd do good to change your 
style.


Andrei
Nov 19 2013
next sibling parent reply "John Colvin" <john.loughran.colvin gmail.com> writes:
On Tuesday, 19 November 2013 at 18:29:57 UTC, Andrei Alexandrescu 
wrote:

 I'll allow myself a piece of advice - the density of e.g. 
 https://github.com/jacob-carlborg/dstep/blob/master/dstep/translat
r/Translator.d#L251 
 is low enough to make it career limiting.


Doesn't look *that* sparse to me... I think the 8-space 
indentation give a false perspective. E.g. this is actually 1 
line longer, but looks denser:

package string translateFunction (string result, string name, 
Parameter[] parameters, bool variadic, String context)
{
     context ~= result ~ ' ' ~ name ~ " (";

     string[] params;
     params.reserve(parameters.length);

     foreach (param ; parameters)
     {
         string p;

         version(D1)
         {
             p ~= param.type;
         }
         else
         {
             if (param.isConst)
             {
                 p ~= "const(" ~ param.type; ~ ')';
             }
             else
             {
                 p ~= param.type;
             }
         }
	
         if (param.name.any)
         {
             p ~= " " ~ translateIdentifier(param.name);
         }
         params ~= p;
     }

     if (variadic)
     {
         params ~= "...";
     }

     context ~= params.join(", ") ~ ')';

     return context.data;
}
Nov 19 2013
parent reply Jacob Carlborg <doob me.com> writes:
On 2013-11-19 20:03, John Colvin wrote:


 Doesn't look *that* sparse to me... I think the 8-space indentation give
 a false perspective. E.g. this is actually 1 line longer, but looks denser:


I'm using one tab as indentation. It's Github formatting it like that.

-- 
/Jacob Carlborg
Nov 19 2013
parent reply Walter Bright <newshound2 digitalmars.com> writes:
On 11/19/2013 12:19 PM, Jacob Carlborg wrote:

 On 2013-11-19 20:03, John Colvin wrote:


 Doesn't look *that* sparse to me... I think the 8-space indentation give
 a false perspective. E.g. this is actually 1 line longer, but looks denser:


 I'm using one tab as indentation. It's Github formatting it like that.


I've abandoned using tabs as every text rendering engine interprets them 
differently.
Nov 19 2013
parent Marco Leise <Marco.Leise gmx.de> writes:
Am Tue, 19 Nov 2013 12:27:43 -0800
schrieb Walter Bright <newshound2 digitalmars.com>:


 I've abandoned using tabs as every text rendering engine interprets them 
 differently.


Yes, that's one of the selling points of using tabs, as
they all have a tab width setting you can customize. :)
Nov 20 2013
prev sibling parent reply Jacob Carlborg <doob me.com> writes:
On 2013-11-19 19:29, Andrei Alexandrescu wrote:


 That's not an expression, it's a statement - more precisely an
 expression statement. (The semicolon makes it so.) By the rules you
 incorrectly stated, there should be an empty line after it.


I'm sorry for not know every minor detail of the language.


 I'll allow myself a piece of advice - the density of e.g.
 https://github.com/jacob-carlborg/dstep/blob/master/dstep/translator/Translator.d#L251
 is low enough to make it career limiting. You'd do good to change your
 style.


Are you serious? Anyone caring about that doesn't know what he/she is doing.

-- 
/Jacob Carlborg
Nov 19 2013
parent reply Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
On 11/19/13 12:17 PM, Jacob Carlborg wrote:

 On 2013-11-19 19:29, Andrei Alexandrescu wrote:


 That's not an expression, it's a statement - more precisely an
 expression statement. (The semicolon makes it so.) By the rules you
 incorrectly stated, there should be an empty line after it.


 I'm sorry for not know every minor detail of the language.


It's not a minor detail. Besides, people took time to explain this to 
you, and the right response is to integrate that information, not to 
demean it.


 I'll allow myself a piece of advice - the density of e.g.
 https://github.com/jacob-carlborg/dstep/blob/master/dstep/translator/Translator.d#L251

 is low enough to make it career limiting. You'd do good to change your
 style.


 Are you serious? Anyone caring about that doesn't know what he/she is
 doing.


That's exactly my point. The matter of fact is, in a setting where 
people are paid to write code, this kind of minor issue would be settled 
around the first week since hiring. At Facebook for example you'd be 
submitting a phabricator diff (loosely equivalent to a github pull 
request) and our linter will point out we use two spaces for indentation 
instead of tabs, and 80 columns. Then a couple of peers would point out 
that code is about twice as sparse vertically than it should. You'd fix 
these issues for good and that would be that. This has happened quite a 
few times. If, on the other hand, you chose to make a big deal out of 
it, that would be a cultural mismatch that to my knowledge would be 
unprecedented.


Andrei
Nov 19 2013
next sibling parent reply "growler" <growlercab gmail.com> writes:
On Tuesday, 19 November 2013 at 21:21:30 UTC, Andrei Alexandrescu 
wrote:

 On 11/19/13 12:17 PM, Jacob Carlborg wrote:

 On 2013-11-19 19:29, Andrei Alexandrescu wrote:


 That's not an expression, it's a statement - more precisely an
 expression statement. (The semicolon makes it so.) By the 
 rules you
 incorrectly stated, there should be an empty line after it.


 I'm sorry for not know every minor detail of the language.


 It's not a minor detail. Besides, people took time to explain 
 this to you, and the right response is to integrate that 
 information, not to demean it.


 I'll allow myself a piece of advice - the density of e.g.
 https://github.com/jacob-carlborg/dstep/blob/master/dstep/translator/Translator.d#L251

 is low enough to make it career limiting. You'd do good to 
 change your
 style.


 Are you serious? Anyone caring about that doesn't know what 
 he/she is
 doing.


 That's exactly my point. The matter of fact is, in a setting 
 where people are paid to write code, this kind of minor issue 
 would be settled around the first week since hiring. At 
 Facebook for example you'd be submitting a phabricator diff 
 (loosely equivalent to a github pull request) and our linter 
 will point out we use two spaces for indentation instead of 
 tabs, and 80 columns. Then a couple of peers would point out 
 that code is about twice as sparse vertically than it should. 
 You'd fix these issues for good and that would be that. This 
 has happened quite a few times. If, on the other hand, you 
 chose to make a big deal out of it, that would be a cultural 
 mismatch that to my knowledge would be unprecedented.


 Andrei


Man, I thought these debates died out long ago. All that matters 
in style is readability and consistency.

Looking at that code linked above::

1. Is the code formatting consistent? Yes
2. Is the code formatting consistent within the repo? Yes
3. Is the code formatting easily read by a programmer? Yes

Whether you, I, or anyone else like with the style is irrelevant. 
Personally I'm not a fan, but that doesn't make the author a bad 
programmer, nor does it make me right. It simply means:

a) Jacob and I prefer different formatting styles and
b) I need to adapt if I'm to work on dstep.

Now if dstep was written at Facebook then OK, the code would need 
reformatting because it violates point 2. above.

As for Phobos code, insist on tighter style guidelines. No one 
will care as long as it is consistent and readable. Any 
programmer worth anything will just adhere to it happily. Those 
that don't want to are not going to have much to contribute, 
they're more worried about their personal style than advancing 
Phobos and D with great code.

Cheers.
Nov 19 2013
next sibling parent Jacob Carlborg <doob me.com> writes:
On 2013-11-20 02:26, growler wrote:


 Man, I thought these debates died out long ago. All that matters in
 style is readability and consistency.

 Looking at that code linked above::

 1. Is the code formatting consistent? Yes
 2. Is the code formatting consistent within the repo? Yes
 3. Is the code formatting easily read by a programmer? Yes

 Whether you, I, or anyone else like with the style is irrelevant.
 Personally I'm not a fan, but that doesn't make the author a bad
 programmer, nor does it make me right. It simply means:

 a) Jacob and I prefer different formatting styles and
 b) I need to adapt if I'm to work on dstep.

 Now if dstep was written at Facebook then OK, the code would need
 reformatting because it violates point 2. above.

 As for Phobos code, insist on tighter style guidelines. No one will care
 as long as it is consistent and readable. Any programmer worth anything
 will just adhere to it happily. Those that don't want to are not going
 to have much to contribute, they're more worried about their personal
 style than advancing Phobos and D with great code.

 Cheers.


Thank you.

-- 
/Jacob Carlborg
Nov 19 2013
prev sibling parent reply "Dicebot" <public dicebot.lv> writes:
On Wednesday, 20 November 2013 at 01:26:35 UTC, growler wrote:

 ...

 Cheers.


Very true. I sometimes switch between 3 to 4 different code 
styles within a single day when actively working on some D 
projects (internal one at work, Phobos style, vibe.d guidelines + 
one preferred style for random chosen project of the day). Pretty 
much only issue it causes is necessity to switch vim tab/space 
profiles ;) I fully expect any reasonable programmer to be able 
to conform any target code style just by a random call.

There are no really good or bad styles, only consistent and 
inconsistent ones.
Nov 20 2013
parent reply Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
On 11/20/13 5:00 AM, Dicebot wrote:

 On Wednesday, 20 November 2013 at 01:26:35 UTC, growler wrote:

 ...

 Cheers.


 Very true. I sometimes switch between 3 to 4 different code styles
 within a single day when actively working on some D projects (internal
 one at work, Phobos style, vibe.d guidelines + one preferred style for
 random chosen project of the day). Pretty much only issue it causes is
 necessity to switch vim tab/space profiles ;) I fully expect any
 reasonable programmer to be able to conform any target code style just
 by a random call.

 There are no really good or bad styles, only consistent and inconsistent
 ones.


Oh there are consistent styles that are worse than other consistent 
styles of course. Another dimension is there are also styles that are 
more or less frequent.

Andrei
Nov 20 2013
parent reply Ary Borenszweig <ary esperanto.org.ar> writes:
On 11/20/13 1:14 PM, Andrei Alexandrescu wrote:

 On 11/20/13 5:00 AM, Dicebot wrote:

 On Wednesday, 20 November 2013 at 01:26:35 UTC, growler wrote:

 ...

 Cheers.


 Very true. I sometimes switch between 3 to 4 different code styles
 within a single day when actively working on some D projects (internal
 one at work, Phobos style, vibe.d guidelines + one preferred style for
 random chosen project of the day). Pretty much only issue it causes is
 necessity to switch vim tab/space profiles ;) I fully expect any
 reasonable programmer to be able to conform any target code style just
 by a random call.

 There are no really good or bad styles, only consistent and inconsistent
 ones.


 Oh there are consistent styles that are worse than other consistent
 styles of course.


Of course: the ones you don't like.
Nov 20 2013
parent reply Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
On 11/20/13 9:43 AM, Ary Borenszweig wrote:

 On 11/20/13 1:14 PM, Andrei Alexandrescu wrote:

 On 11/20/13 5:00 AM, Dicebot wrote:

 On Wednesday, 20 November 2013 at 01:26:35 UTC, growler wrote:

 ...

 Cheers.


 Very true. I sometimes switch between 3 to 4 different code styles
 within a single day when actively working on some D projects (internal
 one at work, Phobos style, vibe.d guidelines + one preferred style for
 random chosen project of the day). Pretty much only issue it causes is
 necessity to switch vim tab/space profiles ;) I fully expect any
 reasonable programmer to be able to conform any target code style just
 by a random call.

 There are no really good or bad styles, only consistent and inconsistent
 ones.


 Oh there are consistent styles that are worse than other consistent
 styles of course.


 Of course: the ones you don't like.


What I meant is there are consistent styles that are objectively worse. 
Consistency is necessary but not sufficient.

Andrei
Nov 20 2013
next sibling parent reply "Dicebot" <public dicebot.lv> writes:
On Wednesday, 20 November 2013 at 20:06:04 UTC, Andrei 
Alexandrescu wrote:

 What I meant is there are consistent styles that are 
 objectively worse. Consistency is necessary but not sufficient.


And what I meant is this opinion of yours is wrong. Any 
consistent style that is liked by at least one programmer that 
uses it in practice is no worse than any other possible 
consistent style. There is nothing objective about it, pretty 
much as there are not that much common about human perception.
Nov 20 2013
next sibling parent =?UTF-8?B?U2ltZW4gS2rDpnLDpXM=?= <simen.kjaras gmail.com> writes:
On 20.11.2013 21:17, Dicebot wrote:

 On Wednesday, 20 November 2013 at 20:06:04 UTC, Andrei Alexandrescu wrote:

 What I meant is there are consistent styles that are objectively
 worse. Consistency is necessary but not sufficient.


 And what I meant is this opinion of yours is wrong. Any consistent style
 that is liked by at least one programmer that uses it in practice is no
 worse than any other possible consistent style. There is nothing
 objective about it, pretty much as there are not that much common about
 human perception.


I think you misunderstand Andrei here. There are styles that are 
consistent that are not liked by *any* programmers. These styles are of 
course not in use.

A style that says you should use seventeen blank lines between 
functions, and each blank line should have 3 tabs and 2 spaces, 
alternating, may be consistent, but it's also ugly, and I daresay that 
is objective (as in, nobody would disagree, I don't believe in perfect 
objectivity).

A less constructed example may be where a non-programmer manager has 
seen that he cannot understand what his programmers are writing, and 
decides to formulate some coding style to make it easier for him, but 
which severely hamstrings actual programmers. (though I guess in this 
case, one could argue it's subjectively better for him...)

-- 
   Simen
Nov 20 2013
prev sibling next sibling parent reply Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
On 11/20/13 12:17 PM, Dicebot wrote:

 On Wednesday, 20 November 2013 at 20:06:04 UTC, Andrei Alexandrescu wrote:

 What I meant is there are consistent styles that are objectively
 worse. Consistency is necessary but not sufficient.


 And what I meant is this opinion of yours is wrong. Any consistent style
 that is liked by at least one programmer that uses it in practice is no
 worse than any other possible consistent style.


There are a bunch of styles that are consistent yet used by nobody, 
because they are objectively worse. So consistency is not the sole 
requirement. Then you hadn't mentioned "that is liked by at least one 
programmer" which moves the goalposts. Then one programmer can be one 
highly unusual human being, so there is strength in numbers.

Generally I tend to double-check with all-inclusive characterization 
"everybody's doing great" and "agree to disagree" and such. Taken to the 
extreme such a stance does little else than to promote undue relativism. 
So whenever someone asks me to agree to disagree I go back to first 
principles and look for rationale that makes one of the points 
objectively refutable.


Andrei
Nov 20 2013
next sibling parent "growler" <growlercab gmail.com> writes:
On Wednesday, 20 November 2013 at 20:47:10 UTC, Andrei 
Alexandrescu wrote:
[snip]

 There are a bunch of styles that are consistent yet used by 
 nobody, because they are objectively worse. So consistency is 
 not the sole requirement.


[snip]

This is true, one can always be consistently illegible :D

http://www.ioccc.org/1986/bright.c

For me personally I always find switching to FORTRAN very 
jarring. It is interesting because FORTRAN itself is very 
readable, but the style often used and ALLCAPS everywhere make it 
hard to read at first.

However, on the whole I've found consistency often leads to 
legibility.

Cheers.
Nov 20 2013
prev sibling parent reply "Dicebot" <public dicebot.lv> writes:
On Wednesday, 20 November 2013 at 20:47:10 UTC, Andrei 
Alexandrescu wrote:

 There are a bunch of styles that are consistent yet used by 
 nobody, because they are objectively worse. So consistency is 
 not the sole requirement. Then you hadn't mentioned "that is 
 liked by at least one programmer" which moves the goalposts. 
 Then one programmer can be one highly unusual human being, so 
 there is strength in numbers.


You see, the whole definition of term "objective comparison" 
implies comparison by  traits other than actual usage. The very 
moment when you refer to something being worse because no one (or 
no one in sane mind) uses it, comparison moves into area of 
subjectivity. Not your specific subjectivity but subjectivity as 
a general evaluation approach.

I am not arguing about the fact that some code styles are widely 
recognized as better ones. I oppose using the term "objectivity" 
in that context, consider it etymological nitpicking.
Nov 20 2013
next sibling parent reply Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
On 11/20/13 3:45 PM, Dicebot wrote:

 On Wednesday, 20 November 2013 at 20:47:10 UTC, Andrei Alexandrescu wrote:

 There are a bunch of styles that are consistent yet used by nobody,
 because they are objectively worse. So consistency is not the sole
 requirement. Then you hadn't mentioned "that is liked by at least one
 programmer" which moves the goalposts. Then one programmer can be one
 highly unusual human being, so there is strength in numbers.


 You see, the whole definition of term "objective comparison" implies
 comparison by  traits other than actual usage.


No, because the quantity being measured is "worse".

Andrei
Nov 20 2013
next sibling parent reply "Dicebot" <public dicebot.lv> writes:
On Wednesday, 20 November 2013 at 23:57:40 UTC, Andrei 
Alexandrescu wrote:

 No, because the quantity being measured is "worse".

 Andrei


If it would have been an inherent well-defined trait of an object 
itself, it would have been objective comparison. As this quantity 
is defined purely (and no real measurable traits other than 
consistency were proposed) by observing subject, it can't be an 
objective measurement and this objective comparison.

Objective comparison is something like comparing average amount 
of extra symbols per lexer token between two code styles.
Nov 20 2013
parent reply Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
On 11/20/13 4:03 PM, Dicebot wrote:

 On Wednesday, 20 November 2013 at 23:57:40 UTC, Andrei Alexandrescu wrote:

 No, because the quantity being measured is "worse".

 Andrei


 If it would have been an inherent well-defined trait of an object
 itself, it would have been objective comparison. As this quantity is
 defined purely (and no real measurable traits other than consistency
 were proposed) by observing subject, it can't be an objective
 measurement and this objective comparison.


In this context "objectively worse" means "all reasonable people would 
say it's worse".

Andrei
Nov 20 2013
next sibling parent reply "Dicebot" <public dicebot.lv> writes:
On Thursday, 21 November 2013 at 01:45:46 UTC, Andrei 
Alexandrescu wrote:

 In this context "objectively worse" means "all reasonable 
 people would say it's worse".

 Andrei


Which is exactly hidden changing of word meaning to push own 
opinion which disgusts me.

http://www.oxforddictionaries.com/definition/english/objective

"not influenced by personal feelings or opinions in considering 
and representing facts; not dependent on the mind for existence; 
actual:"

"opinion" pretty much annihilates "objectivity" when used in same 
sentence.

If statistical opinion sounds "objective" enough for you, I may 
propose to start changing laws of Physics by voting. Why would it 
object if all reasonable people say so?
Nov 20 2013
next sibling parent Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
On 11/20/13 6:02 PM, Dicebot wrote:

 On Thursday, 21 November 2013 at 01:45:46 UTC, Andrei Alexandrescu wrote:

 In this context "objectively worse" means "all reasonable people would
 say it's worse".

 Andrei


 Which is exactly hidden changing of word meaning to push own opinion
 which disgusts me.


No reason to escalate.


 http://www.oxforddictionaries.com/definition/english/objective

 "not influenced by personal feelings or opinions in considering and
 representing facts; not dependent on the mind for existence; actual:"


Yah, and it's a fact that certain coding styles would not be used in 
spite of them being consistent.


 "opinion" pretty much annihilates "objectivity" when used in same sentence.


It does, but this is not about opinions.


 If statistical opinion sounds "objective" enough for you, I may propose
 to start changing laws of Physics by voting. Why would it object if all
 reasonable people say so?


... and the strawman is also uncalled for.


Andrei
Nov 20 2013
prev sibling parent "xenon325" <anm programmer.net> writes:
On Thursday, 21 November 2013 at 02:02:08 UTC, Dicebot wrote:

 On Thursday, 21 November 2013 at 01:45:46 UTC, Andrei 
 Alexandrescu wrote:

 In this context "objectively worse" means "all reasonable 
 people would say it's worse".

 Andrei




A sample of code from my very first job (needless to say that 
wasn't software company):

if (...) statements1; else
statements2;

yes, not indentation and it was used pretty consistently, and I'm 
not kidding!
To make things worse, in the code fragment I was debugging 
`statements1` exceeded screen width, so I have not seen `else`.

It was on my very first day, and after discovering this nightmare 
I spent all the rest of the day searching for code pretty-printer.
At that moment I didn't even know how these class of programs 
called or if they exist at all, but debugging code written like 
that would be unbearable.

Now, Michael, please, go and show me *a single* experienced 
programmer who wouldn't call this coding style a disaster. 
DISASTER.

I think I understand your stance - adherence to scientific method 
etc and that's great, but there is a limit to it.

(Once again, I'm not exaggerating at all. There were other "nice" 
things there, like behemoth method 8k lines long, code 
duplication all over the place, sometimes tens times)
Nov 20 2013
prev sibling parent reply Walter Bright <newshound2 digitalmars.com> writes:
On 11/20/2013 5:45 PM, Andrei Alexandrescu wrote:

 In this context "objectively worse" means "all reasonable people would say it's
 worse".


It could also be objectively measured as:

1. results in more bugs

2. takes people longer to get up to speed with code written that way
Nov 20 2013
parent reply "Dicebot" <public dicebot.lv> writes:
On Thursday, 21 November 2013 at 03:15:16 UTC, Walter Bright 
wrote:

 On 11/20/2013 5:45 PM, Andrei Alexandrescu wrote:

 In this context "objectively worse" means "all reasonable 
 people would say it's
 worse".


 It could also be objectively measured as:

 1. results in more bugs

 2. takes people longer to get up to speed with code written 
 that way


You need to prove first that it results in more bugs because of 
inherent code style traits, not preferences of target audience. 
Otherwise you can have a potential code style which is 
objectively good for one measured group of programmers and 
objectively bad for other. Which contradicts the definition of 
"objectivity".

There is nothing terrible about being only subjectively worse / 
better. Code style exists only to help people and it is 
completely natural that its applicability is evaluated in people 
context. But that is a different thing with different name.

Same with your plane example - as described, one case wasn't 
objectively worse than another (or probably was but it wasn't 
checked). But as it is all about pilots (who are very alive 
subjects), subjective comparison is important enough to make the 
change and resulted in real practical improvement.
Nov 21 2013
parent Walter Bright <newshound2 digitalmars.com> writes:
On 11/21/2013 6:10 AM, Dicebot wrote:

 You need to prove first that it results in more bugs because of inherent code
 style traits, not preferences of target audience.


You're saying that human factors engineering is purely a matter of personal 
preference. IT IS NOT. The way the mind works is not random.



 Same with your plane example - as described, one case wasn't objectively worse
 than another (or probably was but it wasn't checked).


Yes it was checked. People crashed. It was objectively worse. It's not a matter 
of preference.



 But as it is all about
 pilots (who are very alive subjects), subjective comparison is important enough
 to make the change and resulted in real practical improvement.


A real practical improvement is quite objective and measurable - fewer crashes. 
It's not happenstance that airline travel is far, far safer now. An awful lot
of 
crashes were caused by pilot confusion - and every one is analyzed and designs 
are changed and tested to eliminate that confusion.

The objective results are obvious.
Nov 21 2013
prev sibling parent "deadalnix" <deadalnix gmail.com> writes:
On Wednesday, 20 November 2013 at 23:57:40 UTC, Andrei 
Alexandrescu wrote:

 On 11/20/13 3:45 PM, Dicebot wrote:

 On Wednesday, 20 November 2013 at 20:47:10 UTC, Andrei 
 Alexandrescu wrote:

 There are a bunch of styles that are consistent yet used by 
 nobody,
 because they are objectively worse. So consistency is not the 
 sole
 requirement. Then you hadn't mentioned "that is liked by at 
 least one
 programmer" which moves the goalposts. Then one programmer 
 can be one
 highly unusual human being, so there is strength in numbers.


 You see, the whole definition of term "objective comparison" 
 implies
 comparison by  traits other than actual usage.


 No, because the quantity being measured is "worse".

 Andrei


Ho please, we aren't in a schoolyard.
Nov 20 2013
prev sibling parent Walter Bright <newshound2 digitalmars.com> writes:
On 11/20/2013 3:45 PM, Dicebot wrote:

 I oppose using the term "objectivity" in that context, consider it
 etymological nitpicking.


To reiterate my analogy, some cockpit control designs caused more crashes. This 
is objective fact. It isn't a matter of the pilots "liking" or "disliking" one 
or the other.
Nov 20 2013
prev sibling parent reply Walter Bright <newshound2 digitalmars.com> writes:
On 11/20/2013 12:17 PM, Dicebot wrote:

 On Wednesday, 20 November 2013 at 20:06:04 UTC, Andrei Alexandrescu wrote:

 What I meant is there are consistent styles that are objectively worse.
 Consistency is necessary but not sufficient.


 And what I meant is this opinion of yours is wrong. Any consistent style that
is
 liked by at least one programmer that uses it in practice is no worse than any
 other possible consistent style. There is nothing objective about it, pretty
 much as there are not that much common about human perception.


I'm going to very strongly disagree with this.

And I'll use a hoary analogy - airplane cockpit design. It doesn't matter how 
consistent it is - there are an awful lot of cockpit design features that have 
caused crashes, and the fix was changing the feature. These misfeatures were
not 
technically faulty nor were they inconsistent. But they were still bad.

I'll give one example. Warning horns for things that went wrong were good 
features. So good, in fact, that designers added more and more horns. Each horn 
had a different sound signature. The horns were consistent, and worked 
faultlessly. Unfortunately, the plethora of horns caused pilots to get confused 
under stress, and they'd react to solve the wrong problem.

The solution was to replace the horn's siren with a voice saying what was
wrong, 
such as "fire engine no. 2" and "pull up". This worked BETTER.

I see no difference with coding styles. Some are better, some are worse. As 
Andrei stated, consistency is not sufficient.

For example, no indenting at all. Consistent? Yes. Bad? Yes. Case closed!
Nov 20 2013
parent "MattCoder" <mattcoder hotmail.com> writes:
On Thursday, 21 November 2013 at 03:10:17 UTC, Walter Bright 
wrote:

 ...
 The solution was to replace the horn's siren with a voice 
 saying what was wrong, such as "fire engine no. 2" and "pull 
 up". This worked BETTER.

 I see no difference with coding styles. Some are better, some 
 are worse. As Andrei stated, consistency is not sufficient.

 For example, no indenting at all. Consistent? Yes. Bad? Yes. 
 Case closed!


I strongly agree with Walter on this one.
Nov 21 2013
prev sibling parent reply "Jonathan M Davis" <jmdavisProg gmx.com> writes:
On Wednesday, November 20, 2013 12:06:04 Andrei Alexandrescu wrote:

 What I meant is there are consistent styles that are objectively worse.
 Consistency is necessary but not sufficient.


I agree, but you do come across sometimes as seeming to think that some styles 
are objectively worse when plenty of other folks would disagree with you (e.g. 
with regards to how much vertical whitespace is reasonable). I think that in 
most cases, style ends up being very subjective and not objective at all.

IIRC, Walter linked to a blog post a few months back about someone talking 
about what good coding style looked like, and they were talking about the code 
for Doom 3 and how beautiful it was, and yet when they showed what it looked 
like, many people thought that the coding style was absolutely horrible. I 
think that it's quite clear that what people consider to look good in terms of 
coding style varies quite a bit.

So, while some cases are clearly objectively bad (e.g. all of the code on one 
line), I think that you're going to have a hard time arguing it objectively in 
most cases - especially when it comes to legibility, since different people
find 
different styles to be more legible, and what's more legible often comes down 
to what you're used to seeing rather than anything particularly objective.

- Jonathan M Davis
Nov 20 2013
next sibling parent Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
On 11/20/13 12:23 PM, Jonathan M Davis wrote:

 On Wednesday, November 20, 2013 12:06:04 Andrei Alexandrescu wrote:

 What I meant is there are consistent styles that are objectively worse.
 Consistency is necessary but not sufficient.


 I agree, but you do come across sometimes as seeming to think that some styles
 are objectively worse when plenty of other folks would disagree with you (e.g.
 with regards to how much vertical whitespace is reasonable). I think that in
 most cases, style ends up being very subjective and not objective at all.


That I agree with.

Andrei
Nov 20 2013
prev sibling parent reply Walter Bright <newshound2 digitalmars.com> writes:
On 11/20/2013 12:23 PM, Jonathan M Davis wrote:

 So, while some cases are clearly objectively bad (e.g. all of the code on one
 line), I think that you're going to have a hard time arguing it objectively in
 most cases - especially when it comes to legibility, since different people
find
 different styles to be more legible, and what's more legible often comes down
 to what you're used to seeing rather than anything particularly objective.


Legibility can be objective, too.

For example, it is a fact that many fonts do not distinguish O and 0, l and 1. 
It is an objective fact that people have trouble distinguishing them. The JSF 
C++ coding standard, for example, prohibits using an "l" suffix on integral 
literals for that reason.

It's also an objective fact that people have trouble distinguishing:

     verylongisanidentifier

from:

     verulongsianidentifier

It's not just tomayto-tomahto. Human factors research shows that some designs 
are objectively better than others.
Nov 20 2013
parent reply Jonathan M Davis <jmdavisProg gmx.com> writes:
On Wednesday, November 20, 2013 19:22:10 Walter Bright wrote:

 On 11/20/2013 12:23 PM, Jonathan M Davis wrote:

 So, while some cases are clearly objectively bad (e.g. all of the code on
 one line), I think that you're going to have a hard time arguing it
 objectively in most cases - especially when it comes to legibility, since
 different people find different styles to be more legible, and what's
 more legible often comes down to what you're used to seeing rather than
 anything particularly objective.


 Legibility can be objective, too.
 
 For example, it is a fact that many fonts do not distinguish O and 0, l and
 1. It is an objective fact that people have trouble distinguishing them.
 The JSF C++ coding standard, for example, prohibits using an "l" suffix on
 integral literals for that reason.
 
 It's also an objective fact that people have trouble distinguishing:
 
      verylongisanidentifier
 
 from:
 
      verulongsianidentifier
 
 It's not just tomayto-tomahto. Human factors research shows that some
 designs are objectively better than others.


Definitely, but almost all arguments over coding style seem to be very 
subjective even when some people try and claim that some of the issues are 
objective. Most style choices which are objectively bad don't even ever get 
made precisely because they're objectively bad. There are of course 
exceptions, but I've rarely seen style arguments that are actually objective, 
and it's not uncommon to have people with drastically different opinions as to 
what looks good and who think that it should be obvious to everyone that what 
they think looks good looks good and that the other style looks horrible.

And I've run into plenty of cases where one developer thinks that a particular 
coding style is much easier to read, which is in complete contrast with what 
another developer thought was legible (how many parens to use and where being 
a prime example of that). So, it at least almost always seems like what's 
considered be a good, legible style is very subjective.

- Jonathan M Davis
Nov 20 2013
next sibling parent Walter Bright <newshound2 digitalmars.com> writes:
On 11/20/2013 10:35 PM, Jonathan M Davis wrote:

 Definitely, but almost all arguments over coding style seem to be very
 subjective even when some people try and claim that some of the issues are
 objective.


I've run across actual research on aspects of this now and then, and real 
measurable statistics show one way is better than another (better meaning 
produces fewer perceptual mistakes).

It's not dissimilar to A/B testing often done on web shopping pages.



 Most style choices which are objectively bad don't even ever get
 made precisely because they're objectively bad. There are of course
 exceptions, but I've rarely seen style arguments that are actually objective,
 and it's not uncommon to have people with drastically different opinions as to
 what looks good and who think that it should be obvious to everyone that what
 they think looks good looks good and that the other style looks horrible.


Just because A/B testing isn't done doesn't mean it's subjective.



 And I've run into plenty of cases where one developer thinks that a particular
 coding style is much easier to read, which is in complete contrast with what
 another developer thought was legible (how many parens to use and where being
 a prime example of that). So, it at least almost always seems like what's
 considered be a good, legible style is very subjective.


I bet if testing was done on it, a clear difference would emerge.

Just because two developers argue about it doesn't make it subjective. For 
example, we can both argue about where a program is expending its time, and we 
can both be wrong if we actually profile it and see.

The way humans perceive information is not random. Human factors engineering is 
not junk science. There's a reason lower case letters exist, it is not merely 
personal preference. It's objectively easier to read.
Nov 21 2013
prev sibling parent "deadalnix" <deadalnix gmail.com> writes:
On Thursday, 21 November 2013 at 06:35:35 UTC, Jonathan M Davis 
wrote:

 Definitely, but almost all arguments over coding style seem to 
 be very
 subjective even when some people try and claim that some of the 
 issues are
 objective. Most style choices which are objectively bad don't 
 even ever get
 made precisely because they're objectively bad. There are of 
 course
 exceptions, but I've rarely seen style arguments that are 
 actually objective,
 and it's not uncommon to have people with drastically different 
 opinions as to
 what looks good and who think that it should be obvious to 
 everyone that what
 they think looks good looks good and that the other style looks 
 horrible.

 And I've run into plenty of cases where one developer thinks 
 that a particular
 coding style is much easier to read, which is in complete 
 contrast with what
 another developer thought was legible (how many parens to use 
 and where being
 a prime example of that). So, it at least almost always seems 
 like what's
 considered be a good, legible style is very subjective.

 - Jonathan M Davis


It is because it is really context sensitive. Let me take a 
practical example.

function foo()
{
   return
   {
      foo: bar
   }
}

What does this JS sample returns ? Answer is : undefined 
(undefined is a thing in JS, not talking about undefined 
behavior). It is undefined because an semicolon is implicitly 
inserted after return.

It can take place in much more subtle forms.

How does it affect my D style ? It is quite simple, I integrated 
in JS that I should never ever ever put the brace on the next 
line. That is the worse idea ever. My work involve to do some JS, 
so I want to reduce as much as possible the cost of context 
switching and adopt a style that is consistent.

That is why I hate putting brace on the next line.

Indeed, in most language you'll find no objective reason why it 
is good or bad. But in JS you have an objective good reason to do 
it that way. And, as it reduce the cost of context switching, it 
make sense to do it in other languages (JS is not going to 
disappear soon).

Obviously, a dev that never do any JS will find this futile. 
Because he/she do not have to pay the same cost.
Nov 22 2013
prev sibling next sibling parent reply John J <john.joyus gmail.com> writes:
On 11/19/2013 04:21 PM, Andrei Alexandrescu wrote:

 On 11/19/13 12:17 PM, Jacob Carlborg wrote:

 On 2013-11-19 19:29, Andrei Alexandrescu wrote:


 That's not an expression, it's a statement - more precisely an
 expression statement. (The semicolon makes it so.) By the rules you
 incorrectly stated, there should be an empty line after it.


 I'm sorry for not know every minor detail of the language.


 It's not a minor detail. Besides, people took time to explain this to
 you, and the right response is to integrate that information, not to
 demean it.


 I'll allow myself a piece of advice - the density of e.g.
 https://github.com/jacob-carlborg/dstep/blob/master/dstep/translator/Translator.d#L251


 is low enough to make it career limiting. You'd do good to change your
 style.


 Are you serious? Anyone caring about that doesn't know what he/she is
 doing.


 That's exactly my point. The matter of fact is, in a setting where
 people are paid to write code, this kind of minor issue would be settled
 around the first week since hiring. At Facebook for example you'd be
 submitting a phabricator diff (loosely equivalent to a github pull
 request) and our linter will point out we use two spaces for indentation
 instead of tabs, and 80 columns. Then a couple of peers would point out
 that code is about twice as sparse vertically than it should. You'd fix
 these issues for good and that would be that. This has happened quite a
 few times. If, on the other hand, you chose to make a big deal out of
 it, that would be a cultural mismatch that to my knowledge would be
 unprecedented.


 Andrei


This is great stuff for every developer to learn, but can someone please 
create (or start creating) a code formatting tool for D?
I guess it's even worth putting some bounties on it, if that helps.
Nov 19 2013
next sibling parent "growler" <growlercab gmail.com> writes:
On Wednesday, 20 November 2013 at 01:30:18 UTC, John J wrote:

 On 11/19/2013 04:21 PM, Andrei Alexandrescu wrote:

 On 11/19/13 12:17 PM, Jacob Carlborg wrote:

 On 2013-11-19 19:29, Andrei Alexandrescu wrote:


 That's not an expression, it's a statement - more precisely 
 an
 expression statement. (The semicolon makes it so.) By the 
 rules you
 incorrectly stated, there should be an empty line after it.


 I'm sorry for not know every minor detail of the language.


 It's not a minor detail. Besides, people took time to explain 
 this to
 you, and the right response is to integrate that information, 
 not to
 demean it.


 I'll allow myself a piece of advice - the density of e.g.
 https://github.com/jacob-carlborg/dstep/blob/master/dstep/translator/Translator.d#L251


 is low enough to make it career limiting. You'd do good to 
 change your
 style.


 Are you serious? Anyone caring about that doesn't know what 
 he/she is
 doing.


 That's exactly my point. The matter of fact is, in a setting 
 where
 people are paid to write code, this kind of minor issue would 
 be settled
 around the first week since hiring. At Facebook for example 
 you'd be
 submitting a phabricator diff (loosely equivalent to a github 
 pull
 request) and our linter will point out we use two spaces for 
 indentation
 instead of tabs, and 80 columns. Then a couple of peers would 
 point out
 that code is about twice as sparse vertically than it should. 
 You'd fix
 these issues for good and that would be that. This has 
 happened quite a
 few times. If, on the other hand, you chose to make a big deal 
 out of
 it, that would be a cultural mismatch that to my knowledge 
 would be
 unprecedented.


 Andrei


 This is great stuff for every developer to learn, but can 
 someone please create (or start creating) a code formatting 
 tool for D?
 I guess it's even worth putting some bounties on it, if that 
 helps.


I find astyle works very well for D.

http://astyle.sourceforge.net/
Nov 19 2013
prev sibling parent reply "H. S. Teoh" <hsteoh quickfur.ath.cx> writes:
On Tue, Nov 19, 2013 at 08:30:18PM -0500, John J wrote:

 On 11/19/2013 04:21 PM, Andrei Alexandrescu wrote:


[...]

That's exactly my point. The matter of fact is, in a setting where
people are paid to write code, this kind of minor issue would be
settled around the first week since hiring. At Facebook for example
you'd be submitting a phabricator diff (loosely equivalent to a
github pull request) and our linter will point out we use two spaces
for indentation instead of tabs, and 80 columns. Then a couple of
peers would point out that code is about twice as sparse vertically
than it should. You'd fix these issues for good and that would be
that. This has happened quite a few times. If, on the other hand, you
chose to make a big deal out of it, that would be a cultural mismatch
that to my knowledge would be unprecedented.




[...]

 This is great stuff for every developer to learn, but can someone
 please create (or start creating) a code formatting tool for D?
 I guess it's even worth putting some bounties on it, if that helps.


The main holdup is the lack of a ready-made, official D lexer/parser.
Nobody wants to manually maintain their own version of the D
lexer/parser given how fast the language is still developing (plus, it
sucks to have to keep fixing lexing/parsing bugs just because DMD's
lexer/parser doesn't quite work the way you thought it would).

Once we have this in place, I'm quite confident that a lot of nice D
tools would spring up. Pretty-printing is, after all, one of the
simplest uses of lexers/parsers.


T

-- 
One reason that few people are aware there are programs running the
internet is that they never crash in any significant way: the free
software underlying the internet is reliable to the point of
invisibility. -- Glyn Moody, from the article "Giving it all away"
Nov 19 2013
parent reply Ary Borenszweig <ary esperanto.org.ar> writes:
On 11/19/13 10:39 PM, H. S. Teoh wrote:

 On Tue, Nov 19, 2013 at 08:30:18PM -0500, John J wrote:

 On 11/19/2013 04:21 PM, Andrei Alexandrescu wrote:


 [...]

 That's exactly my point. The matter of fact is, in a setting where
 people are paid to write code, this kind of minor issue would be
 settled around the first week since hiring. At Facebook for example
 you'd be submitting a phabricator diff (loosely equivalent to a
 github pull request) and our linter will point out we use two spaces
 for indentation instead of tabs, and 80 columns. Then a couple of
 peers would point out that code is about twice as sparse vertically
 than it should. You'd fix these issues for good and that would be
 that. This has happened quite a few times. If, on the other hand, you
 chose to make a big deal out of it, that would be a cultural mismatch
 that to my knowledge would be unprecedented.




 [...]

 This is great stuff for every developer to learn, but can someone
 please create (or start creating) a code formatting tool for D?
 I guess it's even worth putting some bounties on it, if that helps.


 The main holdup is the lack of a ready-made, official D lexer/parser.
 Nobody wants to manually maintain their own version of the D
 lexer/parser given how fast the language is still developing (plus, it
 sucks to have to keep fixing lexing/parsing bugs just because DMD's
 lexer/parser doesn't quite work the way you thought it would).

 Once we have this in place, I'm quite confident that a lot of nice D
 tools would spring up. Pretty-printing is, after all, one of the
 simplest uses of lexers/parsers.


One day you should all stop doing what you are doing and focus on 
bootstrapping the compiler. Then you will have a lexer/parser that's 
always consistent with the language. And you will have a very big 
program that will make it harder to introduce bugs in next versions. And 
it will show you where you need improvements to the language.

It's really sad that D is a system's programming language but its 
compiler is written in another language.

The strategy I would use is:

1. Copy the code as-is from C++ to D.
2. Make the smallest changes to make it work, even if it doesn't look 
like D code.

At this point, you will have a working compiler in D and a lexer/parser 
as libraries, that are always in-sync with the language.

Then:

3. Refactor the code to make it more D-ish.

Remember: the more you wait for this to happen, the harder it will be.

Just... stop, take some time, and do it.
Nov 20 2013
parent reply "Dicebot" <public dicebot.lv> writes:
On Wednesday, 20 November 2013 at 14:15:17 UTC, Ary Borenszweig 
wrote:

 One day you should all stop doing what you are doing and focus 
 on bootstrapping the compiler.


You'll be surprised - https://github.com/yebblies/magicport2 ;)
( https://github.com/D-Programming-Language/dmd/pull/1980 )

It won't fix the lexer/parser issue on its own as current DMD 
code that does it does not seem to be easily usable as a library. 
Though it will allow to switch compiler to imaginary 
std.d.lexer/std.d.parser to guarantee consistency.
Nov 20 2013
next sibling parent Ary Borenszweig <ary esperanto.org.ar> writes:
On 11/20/13 11:51 AM, Dicebot wrote:

 On Wednesday, 20 November 2013 at 14:15:17 UTC, Ary Borenszweig wrote:

 One day you should all stop doing what you are doing and focus on
 bootstrapping the compiler.


 You'll be surprised - https://github.com/yebblies/magicport2 ;)
 ( https://github.com/D-Programming-Language/dmd/pull/1980 )

 It won't fix the lexer/parser issue on its own as current DMD code that
 does it does not seem to be easily usable as a library. Though it will
 allow to switch compiler to imaginary std.d.lexer/std.d.parser to
 guarantee consistency.


Awesome :-)

Yes, now I remember DMD is not built as a library but as a program (lots 
of global variables).
Nov 20 2013
prev sibling parent "Daniel Murphy" <yebblies nospamgmail.com> writes:
"Dicebot" <public dicebot.lv> wrote in message 
news:nagopvjxtbsaagbsqnxp forum.dlang.org...

 On Wednesday, 20 November 2013 at 14:15:17 UTC, Ary Borenszweig wrote:

 One day you should all stop doing what you are doing and focus on 
 bootstrapping the compiler.


 You'll be surprised - https://github.com/yebblies/magicport2 ;)
 ( https://github.com/D-Programming-Language/dmd/pull/1980 )

 It won't fix the lexer/parser issue on its own as current DMD code that 
 does it does not seem to be easily usable as a library. Though it will 
 allow to switch compiler to imaginary std.d.lexer/std.d.parser to 
 guarantee consistency.


Compiler-as-a-library comes next.
Nov 20 2013
prev sibling parent Jacob Carlborg <doob me.com> writes:
On 2013-11-19 22:21, Andrei Alexandrescu wrote:


 It's not a minor detail. Besides, people took time to explain this to
 you, and the right response is to integrate that information, not to
 demean it.


I think so. I've not magically become a better programmer just because I 
know this information. I can also tell you that a lot of programmers 
have no idea what's the difference between an expression and a 
statement. Many have not even header of those words.

The first person that said that an expression ending with a semicolon is 
a statement was David, and my reply was "Ok, I didn't know that.". 
What's so demeaning about that?


 That's exactly my point. The matter of fact is, in a setting where
 people are paid to write code, this kind of minor issue would be settled
 around the first week since hiring. At Facebook for example you'd be
 submitting a phabricator diff (loosely equivalent to a github pull
 request) and our linter will point out we use two spaces for indentation
 instead of tabs, and 80 columns. Then a couple of peers would point out
 that code is about twice as sparse vertically than it should. You'd fix
 these issues for good and that would be that. This has happened quite a
 few times. If, on the other hand, you chose to make a big deal out of
 it, that would be a cultural mismatch that to my knowledge would be
 unprecedented.


Don't you think I can adapt to a particular style that a company or 
project is using?

I can tell you that I would love to have code reviews like you have at 
Facebook. We have just started using code reviews at my work, it only 
took me three _years_ to get to that.

I can also say that we have lines with over 400 columns, I hate it. But 
it's there because no one cared enough.

I think it would be insulting if someone about to hire me can't see 
through the formatting of my code and base a decision on that. I would 
have no interest in working for someone like that. I don't even want to 
know what other strange things that can be hiding in that company 
behaving like that.

Who said your style is a better one that I should follow.

I should just stop this because it doesn't lead anywhere.

-- 
/Jacob Carlborg
Nov 19 2013
prev sibling parent reply Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
On 11/17/13 11:28 PM, Jonathan M Davis wrote:

 3. Look into API changes that add nothrow, narrower functions to the
 more general ones that may throw.


 In some cases (e.g. format) that's probably a good idea, but I don't think
 that really scales. In some cases, it would be well worth it, whereas in
 others, it would just be better to use try-catch in the few places where you
 know the function won't throw, because it would be quite rare to be able to
 guarantee that it wouldn't throw. It's also not pleasant to have to duplicate
 functions all over the place.


I thought about this some more. So std.datetime accounts for > 1/3 of 
all try statements in Phobos. (It also holds a number of other odd 
records - talking from allegations: largest module in source code, 
requires most memory to build for unittest, most unittest lines per line 
of working code. It's great that these are being worked on.) That must 
mean something. The question is what. It may as well mean that the rest 
of Phobos is sloppy and does not add nothrow in places where it should, 
which would make the rest of it insert a bunch of try/catch in the same 
pattern as std.datetime. An argument based on sense and sensibility 
would suggest that something ought to be done about that idiom in 
std.datetime. But the argument "the more nothrow the merrier" is also 
sensible.

So I think a closer look is warranted here, on a case basis. If the 
conclusion is that such inspection doesn't scale, fine, we've learned 
something. But I think there are more valid learnings to derive from here.

I'll walk through a few samples I gathered below, please don't get 
offended (I know it can feel odd to have one's own code criticized). All 
in good spirit.

1. Consider:

     this(in DateTime dateTime, immutable TimeZone tz = null) nothrow
     {
         try
             this(dateTime, FracSec.from!"hnsecs"(0), tz);
         catch(Exception e)
             assert(0, "FracSec's constructor threw when it shouldn't 
have.");
     }

That's because FracSec.from!"hnsecs"(n) may throw for some values of n. 
To me, this is overkill. Clearly there must be a way to construct a zero 
time interval without much code being executed at all, let alone checks 
and exceptions and whatnot. (In particular FracSec.zero looks like the 
ticket.) Furthermore, having a simpler constructor call a more 
complicated constructor reminds one of the mathematician who throws away 
the water in the kettle to regress to an already solved problem.

2. Consider:

     this(in DateTime dateTime, in FracSec fracSec, immutable TimeZone 
tz = null)
     {
         immutable fracHNSecs = fracSec.hnsecs;
         enforce(fracHNSecs >= 0, new DateTimeException("A SysTime 
cannot have negative fractional seconds."));
         _timezone = tz is null ? LocalTime() : tz;

         try
         {
             immutable dateDiff = (dateTime.date - Date(1, 1, 
1)).total!"hnsecs";
             immutable todDiff = (dateTime.timeOfDay - TimeOfDay(0, 0, 
0)).total!"hnsecs";

             immutable adjustedTime = dateDiff + todDiff + fracHNSecs;
             immutable standardTime = _timezone.tzToUTC(adjustedTime);

             this(standardTime, _timezone);
         }
         catch(Exception e)
         {
             assert(0, "Date, TimeOfDay, or DateTime's constructor threw 
when " ~
                       "it shouldn't have.");
         }
     }

This is not even marked as nothrow (sic!) so the entire try/catch is 
completely meaningless - probably a rote application of the idiom 
without minding its intent. The only difference it would make to the 
user is that a library bug may manifest in slighlty different ways. The 
code has a smartaleck thing about it: "You'd be surprised to see an 
exception here. I, too, would be surprised, and I wanted to make sure I 
tell you about it". The entire try/catch should be removed.

3. Consider:

     this(in Date date, immutable TimeZone tz = null) nothrow
     {
         _timezone = tz is null ? LocalTime() : tz;

         try
         {
             immutable adjustedTime = (date - Date(1, 1, 1)).total!"hnsecs";
             immutable standardTime = _timezone.tzToUTC(adjustedTime);

             this(standardTime, _timezone);
         }
         catch(Exception e)
             assert(0, "Date's constructor through when it shouldn't 
have.");
     }

Here, again, we have a nothrow constructor call a more general one, 
which may throw in general. The intent - maximizing internal reuse 
through forwarding - is noble. The realization - it takes more code to 
reuse than to just set the blessed variables - is a corruption of the 
noble goal.

4. Consider:

      property FracSec fracSec() const nothrow
     {
         try
         {
             auto hnsecs = removeUnitsFromHNSecs!"days"(adjTime);

             if(hnsecs < 0)
                 hnsecs += convert!("hours", "hnsecs")(24);

             hnsecs = removeUnitsFromHNSecs!"seconds"(hnsecs);

             return FracSec.from!"hnsecs"(cast(int)hnsecs);
         }
         catch(Exception e)
             assert(0, "FracSec.from!\"hnsecs\"() threw.");
     }

This is an interesting one. Going to FracSec.from's documentation, we 
see that it throws if the input does not fall within (-1, 1) seconds, 
which means (if I count correctly) hnsec inputs must fall between -10 
million and 10 million. But we already know that we're in good shape 
because we just called removeUnitsFromHNSecs. The problem is the two 
calls are disconnected. That means the API could be improved here: 
combine the two functions by defining a way to get the fractionary 
FracSec from a time value. That will always succeed by definition, so 
we're in good shape.

5. Consider:

     tm toTM() const nothrow
     {
         try
         {
             auto dateTime = cast(DateTime)this;
             tm timeInfo;

             timeInfo.tm_sec = dateTime.second;
             timeInfo.tm_min = dateTime.minute;
             timeInfo.tm_hour = dateTime.hour;
             timeInfo.tm_mday = dateTime.day;
             timeInfo.tm_mon = dateTime.month - 1;
             timeInfo.tm_year = dateTime.year - 1900;
             timeInfo.tm_wday = dateTime.dayOfWeek;
             timeInfo.tm_yday = dateTime.dayOfYear - 1;
             timeInfo.tm_isdst = _timezone.dstInEffect(_stdTime);

             version(Posix)
             {
                 char[] zone = (timeInfo.tm_isdst ? _timezone.dstName : 
_timezone.stdName).dup;
                 zone ~= "\0";

                 timeInfo.tm_gmtoff = cast(int)convert!("hnsecs", 
"seconds")(adjTime - _stdTime);
                 timeInfo.tm_zone = zone.ptr;
             }

             return timeInfo;
         }
         catch(Exception e)
             assert(0, "Either DateTime's constructor threw.");
     }

In fact the assertion message is wrong here. There's no constructor that 
could fail. The problem is with the .dup! That's a bug in .dup for 
char[] - it may indeed throw, but that's an Error not an exception.

(Anyhow, I got curious about tm_zone out of concern for the odd 
allocation. According to 
http://www.cs.utah.edu/dept/old/texinfo/glibc-manual-0.02/library_19.html it's 
a gnu extension and it's a const char*, meaning the user code is not 
supposed to mess with it. Therefore, my guess is some sort of 
memoization here would be in order. But then 
http://www.gsp.com/cgi-bin/man.cgi?topic=mktime suggests it's not const. 
Then 
http://www.eecs.harvard.edu/syrah/vino/release-0.40/man/programref/l
bc/time/ctime.3.txt 
says "he tm_zone field of a returned struct tm points to a static array 
of characters, which will also be overwritten at the next call (and by 
calls to tzset)." That means a static char[3] would be in order. Anyhow, 
more investigation is needed here.)

6. Consider:

     /+ref SysTime+/ void roll(string units)(long value) nothrow
         if(units == "hours" ||
            units == "minutes" ||
            units == "seconds")
     {
         try
         {
             auto hnsecs = adjTime;
             auto days = splitUnitsFromHNSecs!"days"(hnsecs) + 1;

             if(hnsecs < 0)
             {
                 hnsecs += convert!("hours", "hnsecs")(24);
                 --days;
             }

             immutable hour = splitUnitsFromHNSecs!"hours"(hnsecs);
             immutable minute = splitUnitsFromHNSecs!"minutes"(hnsecs);
             immutable second = splitUnitsFromHNSecs!"seconds"(hnsecs);

             auto dateTime = DateTime(Date(cast(int)days), 
TimeOfDay(cast(int)hour, cast(int)minute, cast(int)second));
             dateTime.roll!units(value);
             --days;

             hnsecs += convert!("hours", "hnsecs")(dateTime.hour);
             hnsecs += convert!("minutes", "hnsecs")(dateTime.minute);
             hnsecs += convert!("seconds", "hnsecs")(dateTime.second);

             if(days < 0)
             {
                 hnsecs -= convert!("hours", "hnsecs")(24);
                 ++days;
             }

             immutable newDaysHNSecs = convert!("days", "hnsecs")(days);

             adjTime = newDaysHNSecs + hnsecs;
         }
         catch(Exception e)
             assert(0, "Either DateTime's constructor or TimeOfDay's 
constructor threw.");
     }

Here again the error message is mistaken; the only culprit is 
TimeOfDay's constructor. This prompts a different protest. Note that up 
until the point liable to throw, the input "value" is not involved at 
all, meaning that whatever simple manipulation is done there cannot be 
done without an inefficiency being in the loop. So there's a problem 
with the API design.

7. Consider:

     DateTime opCast(T)() const nothrow
         if(is(Unqual!T == DateTime))
     {
         try
         {
             auto hnsecs = adjTime;
             auto days = splitUnitsFromHNSecs!"days"(hnsecs) + 1;

             if(hnsecs < 0)
             {
                 hnsecs += convert!("hours", "hnsecs")(24);
                 --days;
             }

             immutable hour = splitUnitsFromHNSecs!"hours"(hnsecs);
             immutable minute = splitUnitsFromHNSecs!"minutes"(hnsecs);
             immutable second = getUnitsFromHNSecs!"seconds"(hnsecs);

             return DateTime(Date(cast(int)days), 
TimeOfDay(cast(int)hour, cast(int)minute, cast(int)second));
         }
         catch(Exception e)
             assert(0, "Either DateTime's constructor or TimeOfDay's 
constructor threw.");
     }

This seems to be another instance of the problem at (6). No matter how 
one looks at it, one must admit that the API should be designed such 
that a valid SysTime should convert to the corresponding DateTime 
without much intervening brouhaha.

=============

That 7 issues with as many try/catch instances, i.e. 13% of the total 
54. You may think I cherry-picked them; in fact, they are the FIRST 7 
instances I found by simply searching the file for 'try'. I have no 
reason to believe I won't find similar issues with most or all of them.

I think std.datetime needs a pass with an eye for this idiom and 
obviating it wherever possible.


Andrei
Nov 18 2013
next sibling parent reply Walter Bright <newshound2 digitalmars.com> writes:
On 11/18/2013 11:44 AM, Andrei Alexandrescu wrote:

 1. Consider:

      this(in DateTime dateTime, immutable TimeZone tz = null) nothrow
      {
          try
              this(dateTime, FracSec.from!"hnsecs"(0), tz);
          catch(Exception e)
              assert(0, "FracSec's constructor threw when it shouldn't have.");
      }

 That's because FracSec.from!"hnsecs"(n) may throw for some values of n. To me,
 this is overkill. Clearly there must be a way to construct a zero time interval
 without much code being executed at all, let alone checks and exceptions and
 whatnot. (In particular FracSec.zero looks like the ticket.) Furthermore,
having
 a simpler constructor call a more complicated constructor reminds one of the
 mathematician who throws away the water in the kettle to regress to an already
 solved problem.


I agree. But I'll add that it would be even better to redesign FracSec.from so 
it never throws at all - have it assert(0) for invalid values being passed to
it.

Exceptions should not be thrown for invalid arguments. Data validation should
be 
handled separately.
Nov 18 2013
parent reply "Jonathan M Davis" <jmdavisProg gmx.com> writes:
On Monday, November 18, 2013 12:18:31 Walter Bright wrote:

 I agree. But I'll add that it would be even better to redesign FracSec.from
 so it never throws at all - have it assert(0) for invalid values being
 passed to it.
 
 Exceptions should not be thrown for invalid arguments. Data validation
 should be handled separately.


I think that that depends. In some cases, it would just mean duplicating work 
if the API expected the caller to verify the arguments' validity first. A prime 
example of this would be functions which parse strings. It would be highly 
inefficient for a function like fromISOExtString to assert instead of
enforcing. 
The caller would have to do the exact work that the function has to do in 
order to do the check, so it's just wasteful to expect the caller to ensure 
the validity of the input.

Constructors for types like SysTime or DateTime are in a bit more of a grey 
area in that in most cases what's passed is likely to be valid - e.g. 
hopefully, when someone gives a date, they have some clue that the date is 
valid ahead of time. However, there are cases where the caller really has no 
way of knowing ahead of time whether a date is going to be valid or not - e.g. 
is February 29th a valid date in year X? A function could be provided to check 
the validity ahead of time, but that puts a greater burden on the caller and 
ends up duplicating the checks. It also would have created more code 
duplication in std.datetime in some places, because the checks would have to 
be put in more places rather than having them in a centralized location (e.g. 
Date's constructor can check the validity of the values, and DateTime can just 
take advantage of that rather than having to duplicate the check). I do 
acknowledge however that it's a grey area.

In general, std.datetime takes a defensive programming approach rather than 
the DbC approach with the idea that it's often likely that the input is going 
to be originate from a user or file and that many of the checks have to be done 
regardless, so asking the programmer to do them just adds overhead. In some 
cases, it arguably should have used DbC, but there were enough places that it 
should have been using defensive programming already that it was more 
consistent to use defensive programming in any of the grey areas.

- Jonathan M Davis
Nov 18 2013
parent reply Walter Bright <newshound2 digitalmars.com> writes:
On 11/18/2013 1:16 PM, Jonathan M Davis wrote:

 On Monday, November 18, 2013 12:18:31 Walter Bright wrote:

 I agree. But I'll add that it would be even better to redesign FracSec.from
 so it never throws at all - have it assert(0) for invalid values being
 passed to it.

 Exceptions should not be thrown for invalid arguments. Data validation
 should be handled separately.


 I think that that depends. In some cases, it would just mean duplicating work
 if the API expected the caller to verify the arguments' validity first. A prime
 example of this would be functions which parse strings. It would be highly
 inefficient for a function like fromISOExtString to assert instead of
enforcing.
 The caller would have to do the exact work that the function has to do in
 order to do the check, so it's just wasteful to expect the caller to ensure
 the validity of the input.


I think this is exactly a case where you want to have data validation separate 
from operating on that data. Operating on the data should be efficient, and it 
is not if it must also validate.



 Constructors for types like SysTime or DateTime are in a bit more of a grey
 area in that in most cases what's passed is likely to be valid - e.g.
 hopefully, when someone gives a date, they have some clue that the date is
 valid ahead of time. However, there are cases where the caller really has no
 way of knowing ahead of time whether a date is going to be valid or not - e.g.
 is February 29th a valid date in year X? A function could be provided to check
 the validity ahead of time, but that puts a greater burden on the caller and
 ends up duplicating the checks. It also would have created more code
 duplication in std.datetime in some places, because the checks would have to
 be put in more places rather than having them in a centralized location (e.g.
 Date's constructor can check the validity of the values, and DateTime can just
 take advantage of that rather than having to duplicate the check). I do
 acknowledge however that it's a grey area.


Again, I think data validation must be a separate function. std.datetime is a 
prime example of why.



 In general, std.datetime takes a defensive programming approach rather than
 the DbC approach with the idea that it's often likely that the input is going
 to be originate from a user or file and that many of the checks have to be done
 regardless, so asking the programmer to do them just adds overhead. In some
 cases, it arguably should have used DbC, but there were enough places that it
 should have been using defensive programming already that it was more
 consistent to use defensive programming in any of the grey areas.


I think the fact that there's a lot of code in std.datetime that asserts if the 
underlying functions throw demonstrates my point.

Input data validation and operating on the data should be separate operations, 
not combined.

Functions that operate on data should assert on bad arguments, not throw.
Nov 18 2013
parent reply "Jonathan M Davis" <jmdavisProg gmx.com> writes:
On Monday, November 18, 2013 13:56:39 Walter Bright wrote:

 On 11/18/2013 1:16 PM, Jonathan M Davis wrote:

 On Monday, November 18, 2013 12:18:31 Walter Bright wrote:

 I agree. But I'll add that it would be even better to redesign
 FracSec.from
 so it never throws at all - have it assert(0) for invalid values being
 passed to it.
 
 Exceptions should not be thrown for invalid arguments. Data validation
 should be handled separately.


 
 I think that that depends. In some cases, it would just mean duplicating
 work if the API expected the caller to verify the arguments' validity
 first. A prime example of this would be functions which parse strings. It
 would be highly inefficient for a function like fromISOExtString to
 assert instead of enforcing. The caller would have to do the exact work
 that the function has to do in order to do the check, so it's just
 wasteful to expect the caller to ensure the validity of the input.


 
 I think this is exactly a case where you want to have data validation
 separate from operating on that data. Operating on the data should be
 efficient, and it is not if it must also validate.


But that would just duplicate the validation. You validate by parsing the 
string, and you extract the necessary data from it by parsing it. Validating 
the data first would just double the work - on top of the fact that strings are 
most likely to have come from outside the program rather than having been 
generated internally and then parsed internally. This is exactly the sort of 
case where I think that separate validation makes no sense. Separate 
validation only makes sense when the result is _less_ overhead, not more.

Separate validation is of benefit when the caller can avoid the validation in 
most cases, and the function itself doesn't need to do it, so the validition 
only occurs when the caller needs it. If the validation has to be done anyway, 
then there's no point in having separate validation. It just increases 
overhead.

- Jonathan M Davis
Nov 18 2013
next sibling parent reply Walter Bright <newshound2 digitalmars.com> writes:
I'm glad we're discussing this. There's an important misunderstanding.

On 11/18/2013 2:16 PM, Jonathan M Davis wrote:

 But that would just duplicate the validation. You validate by parsing the
 string, and you extract the necessary data from it by parsing it. Validating
 the data first would just double the work - on top of the fact that strings are
 most likely to have come from outside the program rather than having been
 generated internally and then parsed internally. This is exactly the sort of
 case where I think that separate validation makes no sense. Separate
 validation only makes sense when the result is _less_ overhead, not more.


The point of asserts is that they are supposed to be redundant. (If they were 
not redundant, then they would trip and you'd have a program bug.) Asserts are 
there to detect program bugs, not to validate input data. This is also why the 
optimizer can remove asserts without affecting the meaning of the code.



 Separate validation is of benefit when the caller can avoid the validation in
 most cases, and the function itself doesn't need to do it, so the validition
 only occurs when the caller needs it. If the validation has to be done anyway,
 then there's no point in having separate validation. It just increases
 overhead.


Consider again that std.datetime is chock full of code that requires the 
underlying functions to not throw.

Anytime there is code of the pattern:

    try {
         func();
    } catch (Exception e) {
        assert(0, "func() should not have thrown");
    }

then func() has the wrong API or is being misused.

Again, data validation and bug checking are two very different things and
should 
not be combined.

Note that I have screwed this up myself in the std.uni API.
Nov 18 2013
parent reply "Jonathan M Davis" <jmdavisProg gmx.com> writes:
On Monday, November 18, 2013 14:40:51 Walter Bright wrote:

 I'm glad we're discussing this. There's an important misunderstanding.
 
 On 11/18/2013 2:16 PM, Jonathan M Davis wrote:

 But that would just duplicate the validation. You validate by parsing the
 string, and you extract the necessary data from it by parsing it.
 Validating the data first would just double the work - on top of the fact
 that strings are most likely to have come from outside the program rather
 than having been generated internally and then parsed internally. This is
 exactly the sort of case where I think that separate validation makes no
 sense. Separate validation only makes sense when the result is _less_
 overhead, not more.


 The point of asserts is that they are supposed to be redundant. (If they
 were not redundant, then they would trip and you'd have a program bug.)
 Asserts are there to detect program bugs, not to validate input data. This
 is also why the optimizer can remove asserts without affecting the meaning
 of the code.


I understand this. The problem is that in some cases, in order to do the 
check, you have to do all of the work that the function you're trying to 
protect bad input from has to do anyway - even without it asserting anything. 
So, having a separate function do the checking would just be extra overhead. 
For instance, if you had to parse a string in order to get data out of it, the 
function checking the string's validity would have parse the string out and 
get all of the data out of it, validating the string's format and the data's 
validity in the process, whereas the function that does the actual parsing to 
give you the result (as opposed to checking the input) has to do all of that 
same parsing and data extraction. Maybe, if another function had already 
validated the string, it could avoid a few of the checks, but many of them 
have to be done just to parse the string (e.g. if its format is wrong, you 
can't even get at the data properly, regardless of whether the data is valid 
or not). So, you don't save much in the way of checking if you have a 
validation function, and you add overhead, because the data has to be 
processed twice.

In other cases, validation is as simple as asserting something about the 
input, in which case, it's simple enough to assert within the function (which 
would then go away in -release) and to have a validation function do the 
checking and get no extra overhead, but that's not always the case, and when 
it's not, it makes no sense to me to use DbC. In such cases, defensive 
programming makes far more sense.

Also, if the data _always_ has to be checked (which isn't always the case in 
std.datetime), then it makes no sense to separate the validation from the 
function doing the work.

I think that whether DbC or defensive programming is more appropriate comes 
down primarily to two things:

1. Does the validation need to be part of the function for it to do its job, 
or does doing the validation require doing what the function is going to do 
anyway? If so, the defensive programming makes more sense. If not, then DbC 
makes more sense.

2. Is this function treating its caller as part of the program or as a user? 
If the caller is being treated as part of the program, then DbC tends to make 
sense, as its reasonable to require that the caller knows what the function 
requires and is effectively part of the same code as the function. If the 
caller is being treated as a user (as is often going to be the case with 
libraries), then it's generally better to use defensive programming, because 
it ensures that the function gets and operates on valid input rather than 
resulting in undefined behavior when the caller gives bad input (and unless the 
library is compiled without -release or the function is templated, assertions 
won't do anything to help in a library).

Efficiency tends toward lean towards using DbC, whereas user-friendliness leans 
toward defensive programming. In general, I would use DbC internally to a 
program and defensive programming in a library.

Having validator functions definitely helps with DbC, as it gives the caller a 
way to validate the input when necessary and avoid the validation when it 
isn't. But it puts all the onus on the caller and makes it much, much more 
likely that functions will be misused, and if the function is in a library, 
then the odds are that if validation is done incorrectly by the caller, it'll 
never get checked by the callee, and you'll end up with buggy code with 
undefined behavior.

I think that you bring up good points, but I also don't think that the 
situation is anywhere near as clearcut as you do.

- Jonathan M Davis
Nov 18 2013
parent reply Michel Fortin <michel.fortin michelf.ca> writes:
On 2013-11-18 23:42:13 +0000, "Jonathan M Davis" <jmdavisProg gmx.com> said:


 I understand this. The problem is that in some cases, in order to do the
 check, you have to do all of the work that the function you're trying to
 protect bad input from has to do anyway - even without it asserting anything.
 So, having a separate function do the checking would just be extra overhead.


Very true.

I'll just point out now that you could actually have only one function 
that does one or the other or both depending on template parameters. 
One template parameter is the output sink, which could be a dummy type 
that does nothing or an actual type that saves the data somewhere. 
Another parameter is the error handler, which can be a dummy type that 
does nothing, or it could assert or throw when an error is found. Let 
the optimizer remove the do-nothing code paths that will result.

Now, I really have no idea but that could be overkill in this situation.

-- 
Michel Fortin
michel.fortin michelf.ca
http://michelf.ca
Nov 18 2013
parent "Regan Heath" <regan netmail.co.nz> writes:
On Tue, 19 Nov 2013 03:29:48 -0000, Michel Fortin  
<michel.fortin michelf.ca> wrote:


 On 2013-11-18 23:42:13 +0000, "Jonathan M Davis" <jmdavisProg gmx.com>  
 said:


 I understand this. The problem is that in some cases, in order to do the
 check, you have to do all of the work that the function you're trying to
 protect bad input from has to do anyway - even without it asserting  
 anything.
 So, having a separate function do the checking would just be extra  
 overhead.


 Very true.

 I'll just point out now that you could actually have only one function  
 that does one or the other or both depending on template parameters. One  
 template parameter is the output sink, which could be a dummy type that  
 does nothing or an actual type that saves the data somewhere. Another  
 parameter is the error handler, which can be a dummy type that does  
 nothing, or it could assert or throw when an error is found. Let the  
 optimizer remove the do-nothing code paths that will result.

 Now, I really have no idea but that could be overkill in this situation.


Why use templates at all?  Lets just keep it simple..

You just write /the/ one internal conversion/check function (for each  
case) such that it returns a boolean success status, then.. in release  
mode you call it and throw on false/failure, and in debug mode you call it  
in and assert on false/failure.

R

-- 
Using Opera's revolutionary email client: http://www.opera.com/mail/
Nov 19 2013
prev sibling parent reply "Jesse Phillips" <Jesse.K.Phillips+D gmail.com> writes:
On Monday, 18 November 2013 at 22:17:16 UTC, Jonathan M Davis 
wrote:

 You validate by parsing the
 string, and you extract the necessary data from it by parsing 
 it.


I feel like you and Walter are both saying the same thing.

fromISOString sounds to me like a data validation function. It is 
one that once validated provides a "validated" type, much like 
SortedRange.

I don't think Walter disagrees with that, but isn't calling it 
out specifically. Did I get that right?
Nov 20 2013
parent reply Jonathan M Davis <jmdavisProg gmx.com> writes:
On Thursday, November 21, 2013 08:20:39 Jesse Phillips wrote:

 On Monday, 18 November 2013 at 22:17:16 UTC, Jonathan M Davis
 
 wrote:

 You validate by parsing the
 string, and you extract the necessary data from it by parsing
 it.


 
 I feel like you and Walter are both saying the same thing.
 
 fromISOString sounds to me like a data validation function. It is
 one that once validated provides a "validated" type, much like
 SortedRange.
 
 I don't think Walter disagrees with that, but isn't calling it
 out specifically. Did I get that right?


As I understand it, when Walter is talking about a validator function, _all_ 
it does is validate the input. It doesn't operate on the data beyond doing 
validation - the idea being that you validate that the input has some 
particular property or properties and then all the code that follows can just 
assume that rather than checking it again. For instance, you could have a 
function which validated that a string was valid UTF-8 and then have all code 
that follows just assume that it was valid UTF-8 without ever checking. The 
validator function is a separate function from any of the functions that 
actually do any of the work.

So, to do that with fromISOExtString, you'd have something like

enforce(isValidISOExtString(str)); //validator function
auto data = Date.fromISOExtString(str); //func which actually does the work

and fromISOExtString wouldn't do any validation at all. It would assume that 
the caller had already done the validation - either by calling the validator 
function - isValidISOExtString - or by knowing that it was valid based on 
where it came from (e.g. from toISOExtString).

In many cases, this makes sense from the standpoint of efficiency, because it 
allows you to check once and then avoid checking in the rest of the code 
rather than checking at various points throughout the code and potentially 
duplicating the checks that way. However, in the case of fromISOExtString (as 
well as many other parsing functions), you are essentially required to parse 
the string in order to validate it, meaning that if you had 
isValidISOExtString, and you called it before fromISOExtString, you'd be 
doubling the amount of work. fromISOExtString might be able to avoid a few of 
its checks if it knew that it could assume that the string was already valid, 
but a large portion of the work would have to be done regardless simply 
because that's what it takes to parse the date from the string. As such, 
having a separate validator function doesn't make a lot of sense. Rather, it 
makes more sense to have the function validate its own input rather than 
assume that the caller already validated it. But Walter seems to be arguing in 
favor of having a separate validator function all the time (or very close to 
all the time) - including in this particular case. So, I don't think that 
we're saying the same thing at all.

- Jonathan M Davis
Nov 20 2013
parent reply "Jesse Phillips" <Jesse.K.Phillips+D gmail.com> writes:
On Thursday, 21 November 2013 at 07:44:44 UTC, Jonathan M Davis 
wrote:

 As I understand it, when Walter is talking about a validator 
 function, _all_
 it does is validate the input. It doesn't operate on the data 
 beyond doing validation


I doubt Walter first checks that a file is a valid D program 
before tokenizing. More importantly he probably would not go for 
such a change.

The thing is, ISO String isn't a date, we first need to create a 
date and in doing so we validate it is valid and never have to 
check again.
Nov 21 2013
parent Jonathan M Davis <jmdavisProg gmx.com> writes:
On Thursday, November 21, 2013 16:38:48 Jesse Phillips wrote:

 On Thursday, 21 November 2013 at 07:44:44 UTC, Jonathan M Davis
 
 wrote:

 As I understand it, when Walter is talking about a validator
 function, _all_
 it does is validate the input. It doesn't operate on the data
 beyond doing validation


 
 I doubt Walter first checks that a file is a valid D program
 before tokenizing. More importantly he probably would not go for
 such a change.


Of course not, which makes his argument all the odder. Maybe he misunderstood 
what fromISOExtString does when responding to my argument that it made no 
sense to use a separate validator function for it.


 The thing is, ISO String isn't a date, we first need to create a
 date and in doing so we validate it is valid and never have to
 check again.


Agreed. But Walter is specifically arguing for doing validation in separate 
functions from those that do the work, which would mean having something like 
isValidateISOExtString which could be called fromISOExtString and then 
requiring that the string passed to fromISOExtString be valid or you get 
undefined behavior (in release at least, where there would be no assertions) as 
opposed to doing the checking in fromISOExtString. It's not necessarily a bad 
idea in the general case, but in this particular case, it makes no sense IMHO. 
However, when I argued that, for some reason Walter seemed to think that it 
was a perfect example of a case where there should be a separate validator 
function. So, maybe there's a miscommunicaton with regards to what 
fromISOExtString does.

- Jonathan M Davis
Nov 21 2013
prev sibling next sibling parent Walter Bright <newshound2 digitalmars.com> writes:
On 11/18/2013 11:44 AM, Andrei Alexandrescu wrote:

 Here, again, we have a nothrow constructor call a more general one, which may
 throw in general. The intent - maximizing internal reuse through forwarding -
is
 noble. The realization - it takes more code to reuse than to just set the
 blessed variables - is a corruption of the noble goal.


I also have a design issue with having throwing constructors at all. I know D 
allows them, but I consider it a bad practice.
Nov 18 2013
prev sibling next sibling parent reply Jacob Carlborg <doob me.com> writes:
On 2013-11-18 20:44, Andrei Alexandrescu wrote:


 I thought about this some more. So std.datetime accounts for > 1/3 of
 all try statements in Phobos. (It also holds a number of other odd
 records - talking from allegations: largest module in source code,
 requires most memory to build for unittest, most unittest lines per line
 of working code. It's great that these are being worked on.) That must
 mean something. The question is what. It may as well mean that the rest
 of Phobos is sloppy and does not add nothrow in places where it should,
 which would make the rest of it insert a bunch of try/catch in the same
 pattern as std.datetime. An argument based on sense and sensibility
 would suggest that something ought to be done about that idiom in
 std.datetime. But the argument "the more nothrow the merrier" is also
 sensible.


I really don't understand the point in having some functions thrown and 
others assert.

-- 
/Jacob Carlborg
Nov 18 2013
parent reply Walter Bright <newshound2 digitalmars.com> writes:
On 11/18/2013 1:02 PM, Jacob Carlborg wrote:

 I really don't understand the point in having some functions thrown and others
 assert.


That's a red flag that there is a faulty design somewhere, in this case I think 
it's not having a separation between input data validation and program bug 
detection.
Nov 18 2013
parent reply Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
On 11/18/13 2:04 PM, Walter Bright wrote:

 On 11/18/2013 1:02 PM, Jacob Carlborg wrote:

 I really don't understand the point in having some functions thrown
 and others
 assert.


 That's a red flag that there is a faulty design somewhere, in this case
 I think it's not having a separation between input data validation and
 program bug detection.


The question is what's "program" and what's "input". Consider:

int a = fun();
std.gun(a);

There are two possible takes on this:

1. The standard library is considered part of the user's program, and 
the whole thing is a unit. In that case, passing the wrong int to 
std.gun is an PROGRAM error and 100% blame goes to the programmer who 
wrote the caller code. In that case, assert/assert(0)/contracts are the 
appropriate constructs to be used inside std.gun.

This is the approach taken by the C standard library, which is free to 
do whatever it wants (including crashing the program) upon calls such as 
strlen(NULL) etc.

2. The standard library is a separate entity from the PROGRAM, and as 
far as it cares, any data from the user is INPUT. So the standard 
library with SCRUB the input, in which case enforce() and throwing 
exceptions are appropriate.


extent the newer parts of C++'s standard library.

To claim that one approach is exactly right and the other is exactly 
wrong would miss important insights.


Andrei
Nov 18 2013
next sibling parent Walter Bright <newshound2 digitalmars.com> writes:
On 11/18/2013 2:45 PM, Andrei Alexandrescu wrote:

 There are two possible takes on this:

 1. The standard library is considered part of the user's program, and the whole
 thing is a unit. In that case, passing the wrong int to std.gun is an PROGRAM
 error and 100% blame goes to the programmer who wrote the caller code. In that
 case, assert/assert(0)/contracts are the appropriate constructs to be used
 inside std.gun.

 This is the approach taken by the C standard library, which is free to do
 whatever it wants (including crashing the program) upon calls such as
 strlen(NULL) etc.

 2. The standard library is a separate entity from the PROGRAM, and as far as it
 cares, any data from the user is INPUT. So the standard library with SCRUB the
 input, in which case enforce() and throwing exceptions are appropriate.


 the newer parts of C++'s standard library.

 To claim that one approach is exactly right and the other is exactly wrong
would
 miss important insights.


Or:

3. Input validation and data processing should have separate functions in the 
library.

(The Windows API is a special case - it must regard all input as untrusted, 
unvalidated input, and it must protect Windows itself from malicious input.
This 
is not true of Phobos.)
Nov 18 2013
prev sibling next sibling parent Timon Gehr <timon.gehr gmx.ch> writes:
On 11/18/2013 11:45 PM, Andrei Alexandrescu wrote:

 On 11/18/13 2:04 PM, Walter Bright wrote:

 On 11/18/2013 1:02 PM, Jacob Carlborg wrote:

 I really don't understand the point in having some functions thrown
 and others
 assert.


 That's a red flag that there is a faulty design somewhere, in this case
 I think it's not having a separation between input data validation and
 program bug detection.


 The question is what's "program" and what's "input". Consider:

 int a = fun();
 std.gun(a);

 There are two possible takes on this:

 1. The standard library is considered part of the user's program, and
 the whole thing is a unit. In that case, passing the wrong int to
 std.gun is an PROGRAM error and 100% blame goes to the programmer who
 wrote the caller code. In that case, assert/assert(0)/contracts are the
 appropriate constructs to be used inside std.gun.

 This is the approach taken by the C standard library, which is free to
 do whatever it wants (including crashing the program) upon calls such as
 strlen(NULL) etc.

 2. The standard library is a separate entity from the PROGRAM, and as
 far as it cares, any data from the user is INPUT. So the standard
 library with SCRUB the input, in which case enforce() and throwing
 exceptions are appropriate.


 extent the newer parts of C++'s standard library.

 To claim that one approach is exactly right and the other is exactly
 wrong would miss important insights.
...


1. Has the shortcoming that you may actually get assertion failures.

2. is not wrong but aesthetically unpleasing as it essentially widens 
the interface to allow all possible inputs and extends it with behaviour 
that is useless in order to avoid the situation that the caller does not 
satisfy the interface. There is also:

3. Require both caller and callee to construct a compile-time checkable 
proof that they satisfy their part of the interface.

This is the approach taken by dependently typed programming languages, 
and program verifiers. This subsumes 1. and 2. and is a solution to the 
original problem since in such a setting it can be manually proven that 
a function call will not throw.
Nov 18 2013
prev sibling next sibling parent "Jonathan M Davis" <jmdavisProg gmx.com> writes:
On Monday, November 18, 2013 14:45:54 Andrei Alexandrescu wrote:

 On 11/18/13 2:04 PM, Walter Bright wrote:

 On 11/18/2013 1:02 PM, Jacob Carlborg wrote:

 I really don't understand the point in having some functions thrown
 and others
 assert.


 
 That's a red flag that there is a faulty design somewhere, in this case
 I think it's not having a separation between input data validation and
 program bug detection.


 
 The question is what's "program" and what's "input". Consider:
 
 int a = fun();
 std.gun(a);
 
 There are two possible takes on this:
 
 1. The standard library is considered part of the user's program, and
 the whole thing is a unit. In that case, passing the wrong int to
 std.gun is an PROGRAM error and 100% blame goes to the programmer who
 wrote the caller code. In that case, assert/assert(0)/contracts are the
 appropriate constructs to be used inside std.gun.
 
 This is the approach taken by the C standard library, which is free to
 do whatever it wants (including crashing the program) upon calls such as
 strlen(NULL) etc.
 
 2. The standard library is a separate entity from the PROGRAM, and as
 far as it cares, any data from the user is INPUT. So the standard
 library with SCRUB the input, in which case enforce() and throwing
 exceptions are appropriate.
 

 extent the newer parts of C++'s standard library.
 
 To claim that one approach is exactly right and the other is exactly
 wrong would miss important insights.


+1

- Jonathan M Davis
Nov 18 2013
prev sibling next sibling parent reply Jacob Carlborg <doob me.com> writes:
On 2013-11-18 23:45, Andrei Alexandrescu wrote:


 The question is what's "program" and what's "input". Consider:

 int a = fun();
 std.gun(a);

 There are two possible takes on this:

 1. The standard library is considered part of the user's program, and
 the whole thing is a unit. In that case, passing the wrong int to
 std.gun is an PROGRAM error and 100% blame goes to the programmer who
 wrote the caller code. In that case, assert/assert(0)/contracts are the
 appropriate constructs to be used inside std.gun.

 This is the approach taken by the C standard library, which is free to
 do whatever it wants (including crashing the program) upon calls such as
 strlen(NULL) etc.


In D, we can start by saying that all private and package functions 
should use asserts.


 2. The standard library is a separate entity from the PROGRAM, and as
 far as it cares, any data from the user is INPUT. So the standard
 library with SCRUB the input, in which case enforce() and throwing
 exceptions are appropriate.


 extent the newer parts of C++'s standard library.

 To claim that one approach is exactly right and the other is exactly
 wrong would miss important insights.


I agree, it's not easy to just pick one. Say you have a library that 
queries a database. Is it the user's responsibility to validate any 
input that it doesn't contain SQL injections. Or should the library do that?

I would say that it would be quite cumbersome and verbose to for the 
user to constantly validate the input. If the library didn't validate 
the user would most likely create a wrapper function that does this. 
Most users would want to have a function like this and implement it. 
When we come to this, one can ask, isn't this the job of the library? If 
every user needs it, shouldn't it be put in the library?

-- 
/Jacob Carlborg
Nov 18 2013
parent reply "Jouko Koski" <joukokoskispam101 netti.fi> writes:
"Jacob Carlborg"  wrote:

 I would say that it would be quite cumbersome and verbose to for the 
 user to constantly validate the input.


If the "user level" validation fails, shall we throw or assert or...?

-- 
Jouko


---
avast! Antivirus k=C3=A4ynniss=C3=A4, joten t=C3=A4ss=C3=A4 s=C3=A4hk=C3=B6=
postiviestiss=C3=A4 ei ole viruksia tai haittaohjelmia.
http://www.avast.com
Nov 19 2013
parent reply Jacob Carlborg <doob me.com> writes:
On 2013-11-19 18:11, Jouko Koski wrote:


 If the "user level" validation fails, shall we throw or assert or...?


Throw.

-- 
/Jacob Carlborg
Nov 19 2013
parent reply "Jouko Koski" <joukokoskispam101 netti.fi> writes:
"Jacob Carlborg"  wrote:

 On 2013-11-19 18:11, Jouko Koski wrote:


 If the "user level" validation fails, shall we throw or assert or...?


 Throw.


Ok. What if the "user" is writing a library code which calls other system 
utilities? If the validation fails, is the answer now assert?

I don't have a definite answer. However, I tend to follow the practice that 
all public API should do validation and throw on failures. I use assert for 
catching my own mistakes in more or less internal stuff.

Usually I don't find separate validation API functions very practical. One 
can forget or ignore to do validation, just like one can fail to check errno 
after a call. Of course, validation is necessary and exceptions can assert 
(sic!) this. It might be more of a personal preference, but often I catch 
exceptions only for error reporting purposes. Exception is an indication of 
an unexpected condition in the program. The solution is to fix the program 
to be prepared for the condition, not trying to fix the condition afterwards 
as the exception has already fired. Not all APIs support this approach.

-- 
Jouko
Nov 19 2013
parent Jacob Carlborg <doob me.com> writes:
On 2013-11-20 08:22, Jouko Koski wrote:


 Ok. What if the "user" is writing a library code which calls other
 system utilities? If the validation fails, is the answer now assert?

 I don't have a definite answer. However, I tend to follow the practice
 that all public API should do validation and throw on failures. I use
 assert for catching my own mistakes in more or less internal stuff.


I neither have a definite answer. It depends on what that particular 
function is doing.


 Usually I don't find separate validation API functions very practical.
 One can forget or ignore to do validation, just like one can fail to
 check errno after a call. Of course, validation is necessary and
 exceptions can assert (sic!) this. It might be more of a personal
 preference, but often I catch exceptions only for error reporting
 purposes. Exception is an indication of an unexpected condition in the
 program. The solution is to fix the program to be prepared for the
 condition, not trying to fix the condition afterwards as the exception
 has already fired. Not all APIs support this approach.


I all depends on what API's and what the program is doing. Trying to 
read a file which you don't have permission to read, or doesn't exist 
would probably throw an exception. But that is not an error by the 
programmer. It could be perfectly fine to catch that exception and ask 
the user of the program to try a different file.

-- 
/Jacob Carlborg
Nov 19 2013
prev sibling parent reply Dmitry Olshansky <dmitry.olsh gmail.com> writes:
19-Nov-2013 02:45, Andrei Alexandrescu пишет:

 On 11/18/13 2:04 PM, Walter Bright wrote:

 On 11/18/2013 1:02 PM, Jacob Carlborg wrote:

 I really don't understand the point in having some functions thrown
 and others
 assert.


 That's a red flag that there is a faulty design somewhere, in this case
 I think it's not having a separation between input data validation and
 program bug detection.


 The question is what's "program" and what's "input". Consider:

 int a = fun();
 std.gun(a);

 There are two possible takes on this:

 1. The standard library is considered part of the user's program, and
 the whole thing is a unit. In that case, passing the wrong int to
 std.gun is an PROGRAM error and 100% blame goes to the programmer who
 wrote the caller code. In that case, assert/assert(0)/contracts are the
 appropriate constructs to be used inside std.gun.

 This is the approach taken by the C standard library, which is free to
 do whatever it wants (including crashing the program) upon calls such as
 strlen(NULL) etc.

 2. The standard library is a separate entity from the PROGRAM, and as
 far as it cares, any data from the user is INPUT. So the standard
 library with SCRUB the input, in which case enforce() and throwing
 exceptions are appropriate.


3. The standard library handles all input, the wrong input is eliminated 
as a class by providing meaningful output on error. So called 
soft-errors. An example would be returning a 0xFFFD when decoding a 
broken UTF-8 codepoint. Arguably bad examples include fopen/malloc 
returning 0.

All three cases can and should be judiciously mixed and matched.

It's also interesting to see that some primitives are better be designed 
to work with untrusted input see utf.decode.
While others are designed not. What I mean by _designed_ is that 
encountering wrong inputs in a correct program is unavoidable and to be 
expected. Also (2) and (3) can be constructed on top of (1) by widening 
the interface.

Compare hypothetical:
Date parseDate(string str);
vs:
Date date(Year year, Month month, Day day);

The first better be designed to scrub input, while the second is 
designed to accept verified values (hence Year, Month and not naked ints).


-- 
Dmitry Olshansky
Nov 19 2013
parent "Jonathan M Davis" <jmdavisProg gmx.com> writes:
On Tuesday, November 19, 2013 16:48:30 Dmitry Olshansky wrote:

 It's also interesting to see that some primitives are better be designed
 to work with untrusted input see utf.decode.
 While others are designed not. What I mean by _designed_ is that
 encountering wrong inputs in a correct program is unavoidable and to be
 expected. Also (2) and (3) can be constructed on top of (1) by widening
 the interface.
 
 Compare hypothetical:
 Date parseDate(string str);
 vs:
 Date date(Year year, Month month, Day day);
 
 The first better be designed to scrub input, while the second is
 designed to accept verified values (hence Year, Month and not naked ints).


True, though there can still be invalid days depending on the month and year, 
so it's not the case that there can't be bad input even if you no casting to 
Year, Month, or Day was involved. It's one that I debated on and end up going 
with exceptions, because it was consistent with everything else in 
std.datetime to do so, but it could also be argued that asserting would have 
been better in this case. In contrast, as you indicated, parsing a string for 
a date is clearly a case where exceptions are better.

- Jonathan M Davis
Nov 19 2013
prev sibling parent "Jonathan M Davis" <jmdavisProg gmx.com> writes:
On Monday, November 18, 2013 11:44:46 Andrei Alexandrescu wrote:

 That 7 issues with as many try/catch instances, i.e. 13% of the total
 54. You may think I cherry-picked them; in fact, they are the FIRST 7
 instances I found by simply searching the file for 'try'. I have no
 reason to believe I won't find similar issues with most or all of them.
 
 I think std.datetime needs a pass with an eye for this idiom and
 obviating it wherever possible.


Clearly, I need to take a look at this. I don't think that I ever made an 
attempt to avoid the try-catch-assert(0) idiom. I used exceptions where they 
made sense and then used try-catch-assert(0) where it was required in order to 
mark functions as nothrow when I knew that they logically could/should be 
nothrow. On the whole, I think that the implementaton of std.datetime is good, 
but my primary focus was always on the API, and it was a lot of code for 
people to review, so it's not entirely surprising if some things could/should 
be improved. There were a _lot_ of improvements thanks to the review process, 
but that doesn't mean that everything was caught. It's also the case that 
we've all learned quite a bit in the years since, and the compiler and 
libraries have improved as well, changing what does and doesn't work in some 
cases (e.g. any function with format is currently impure in std.datetime, but 
it now may be able to be pure). So, it should now be possible to improve some 
of std.datetime's implementation beyond what we could do when it was first 
introduced.

Once I've finished splitting std.datetime (which is mostly done, but I've been 
too busy lately to finish quite yet), I'll look into reducing the number of 
instances try-catch-assert(0) in std.datetime as well as looking for any other 
implementation improvements which can be made. And that's one thing that the 
plethora of unit tests makes easier, because the risk of breaking stuff and not 
catching it is fairly low (in fact, Martin Nowak made improvements to a 
function in core.time recently and expressed that he was glad for how much 
easier it made it to catch mistakes in his refactor because of how thorough 
the tests were).

- Jonathan M Davis
Nov 18 2013
prev sibling next sibling parent reply "Daniel Murphy" <yebblies nospamgmail.com> writes:
"Andrei Alexandrescu" <SeeWebsiteForEmail erdani.org> wrote in message 
news:l6ccae$2cmt$1 digitalmars.com...

 1. Fix scope(failure) and then use it.


scope(failure) picks up Errors as well as Exceptions
Nov 18 2013
next sibling parent reply Jonathan M Davis <jmdavisProg gmx.com> writes:
On Monday, November 18, 2013 20:56:47 Daniel Murphy wrote:

 "Andrei Alexandrescu" <SeeWebsiteForEmail erdani.org> wrote in message
 news:l6ccae$2cmt$1 digitalmars.com...
 

 1. Fix scope(failure) and then use it.


 
 scope(failure) picks up Errors as well as Exceptions


Ouch. That's true, which more or less kills that idea - though according to 
Walter, it's not supposed to be guaranteed to do so, and he'd probably prefer 
that it never did. But that's a whole debate in and of itself, and we've had 
it before.

- Jonathan M Davis
Nov 18 2013
parent reply "Daniel Murphy" <yebblies nospamgmail.com> writes:
"Jonathan M Davis" <jmdavisProg gmx.com> wrote in message 
news:mailman.59.1384769025.2552.digitalmars-d puremagic.com...

 On Monday, November 18, 2013 20:56:47 Daniel Murphy wrote:

 "Andrei Alexandrescu" <SeeWebsiteForEmail erdani.org> wrote in message
 news:l6ccae$2cmt$1 digitalmars.com...


 1. Fix scope(failure) and then use it.


 scope(failure) picks up Errors as well as Exceptions


 Ouch. That's true, which more or less kills that idea - though according 
 to
 Walter, it's not supposed to be guaranteed to do so, and he'd probably 
 prefer
 that it never did. But that's a whole debate in and of itself, and we've 
 had
 it before.

 - Jonathan M Davis


Yeah.  On the other hand, if we decide assert(0) means 'assume unreachable' 
we can optimize out the try-catch in release mode, among other things.

try { s } catch { assert(0); } -> s
if (e) assert(0); else s; -> e; s;

etc
Nov 18 2013
next sibling parent Jonathan M Davis <jmdavisProg gmx.com> writes:
On Monday, November 18, 2013 21:16:20 Daniel Murphy wrote:

 Yeah.  On the other hand, if we decide assert(0) means 'assume unreachable'
 we can optimize out the try-catch in release mode, among other things.
 
 try { s } catch { assert(0); } -> s
 if (e) assert(0); else s; -> e; s;
 
 etc


I was hoping that we could do something like that, since that would fix this 
problem as far as optimizations go. It leaves the slight verbosity of the try-
catch, which is a bit annoying, but I wouldn't consider it all that big a 
deal, as nice as it would be to make it less verbose.

- Jonathan M Davis
Nov 18 2013
prev sibling next sibling parent reply Jacob Carlborg <doob me.com> writes:
On 2013-11-18 11:16, Daniel Murphy wrote:


 Yeah.  On the other hand, if we decide assert(0) means 'assume unreachable'
 we can optimize out the try-catch in release mode, among other things.

 try { s } catch { assert(0); } -> s
 if (e) assert(0); else s; -> e; s;


"assume" isn't the same as the compile will do this. Currently the spec 
says "Either AssertError is thrown at runtime if it is reachable, or the 
execution is halted". To me that means an implementation is free to 
throw an AssertError.

-- 
/Jacob Carlborg
Nov 18 2013
parent reply Jonathan M Davis <jmdavisProg gmx.com> writes:
On Monday, November 18, 2013 11:53:50 Jacob Carlborg wrote:

 On 2013-11-18 11:16, Daniel Murphy wrote:

 Yeah.  On the other hand, if we decide assert(0) means 'assume
 unreachable'
 we can optimize out the try-catch in release mode, among other things.
 
 try { s } catch { assert(0); } -> s
 if (e) assert(0); else s; -> e; s;


 
 "assume" isn't the same as the compile will do this. Currently the spec
 says "Either AssertError is thrown at runtime if it is reachable, or the
 execution is halted". To me that means an implementation is free to
 throw an AssertError.


assert(0) is intended specifically for use in cases where a line is supposed to 
be unreachable, and it wouldn't make any sense to use it in any other case, 
because assertion failures are intended to kill the program, and assert(0) 
always fails. It happens that it throws an AssertError in non-release mode in 
order to give you better debug information on failure, and it's a HLT 
instruction in release, but in either case, it's intended to be unreachable 
code.

The spec really should be updated to make it clear that when assertions are 
compiled in, assert(0) throws an AssertError and that when assertions are 
supposed to be compiled out, it becomes a HLT instruction. And if need be, we 
can update the spec to require that try-catches be compiled out when 
assertions are compiled out, and the catch's body only contains an assert(0).

- Jonathan M Davis
Nov 18 2013
parent reply Jacob Carlborg <doob me.com> writes:
On 2013-11-18 12:03, Jonathan M Davis wrote:


 assert(0) is intended specifically for use in cases where a line is supposed to
 be unreachable, and it wouldn't make any sense to use it in any other case,
 because assertion failures are intended to kill the program, and assert(0)
 always fails. It happens that it throws an AssertError in non-release mode in
 order to give you better debug information on failure, and it's a HLT
 instruction in release, but in either case, it's intended to be unreachable
 code.

 The spec really should be updated to make it clear that when assertions are
 compiled in, assert(0) throws an AssertError and that when assertions are
 supposed to be compiled out, it becomes a HLT instruction. And if need be, we
 can update the spec to require that try-catches be compiled out when
 assertions are compiled out, and the catch's body only contains an assert(0).


Does all architectures support the HLT instruction or equivalent? The 
spec explicitly says HLT is used on x86.

-- 
/Jacob Carlborg
Nov 18 2013
next sibling parent Jonathan M Davis <jmdavisProg gmx.com> writes:
On Monday, November 18, 2013 13:23:23 Jacob Carlborg wrote:

 On 2013-11-18 12:03, Jonathan M Davis wrote:

 assert(0) is intended specifically for use in cases where a line is
 supposed to be unreachable, and it wouldn't make any sense to use it in
 any other case, because assertion failures are intended to kill the
 program, and assert(0) always fails. It happens that it throws an
 AssertError in non-release mode in order to give you better debug
 information on failure, and it's a HLT instruction in release, but in
 either case, it's intended to be unreachable code.
 
 The spec really should be updated to make it clear that when assertions
 are
 compiled in, assert(0) throws an AssertError and that when assertions are
 supposed to be compiled out, it becomes a HLT instruction. And if need be,
 we can update the spec to require that try-catches be compiled out when
 assertions are compiled out, and the catch's body only contains an
 assert(0).


 Does all architectures support the HLT instruction or equivalent? The
 spec explicitly says HLT is used on x86.


I don't know. My knowledge of stuff at that level on x86 is already poor. I 
have pretty much zero knowledge about that sort of thing on other 
architectures. The extent of my knowledge in that area tends to be restricted 
to big endian vs little endian. The only reason that I even know that HLT 
exists is thanks to this feature in D.

If it's not possible to require HLT or an equivalent, then it wouldn't make 
sense to put that in the spec, which would be unfortunate, but ideally that 
would be required. However, even if it can't be, that doesn't change the fact 
that assert(0) is intended to indicate unreachable code and doesn't make sense 
for anything else (as I recall, the compiler itself inserts it for that 
purpose at the end of functions in some circumstances to catch missing return 
statements). So, I see no problem with the compiler making optimizations in 
release based on the assumption that assert(0) should never be reached.

- Jonathan M Davis
Nov 18 2013
prev sibling parent reply Walter Bright <newshound2 digitalmars.com> writes:
On 11/18/2013 4:23 AM, Jacob Carlborg wrote:

 Does all architectures support the HLT instruction or equivalent? The spec
 explicitly says HLT is used on x86.


Most architectures I've used supported a HLT, and there's always something the 
back end could construct if it had to, even:

      ...
      call halt
      ...
Nov 18 2013
next sibling parent reply Jacob Carlborg <doob me.com> writes:
On 2013-11-18 21:49, Walter Bright wrote:


 Most architectures I've used supported a HLT, and there's always
 something the back end could construct if it had to, even:

       ...
       call halt
       ...


In that case, would we want it to be legal to throw AssertError instead 
of using the halt instructions? Because that's what the docs says.

-- 
/Jacob Carlborg
Nov 18 2013
next sibling parent Walter Bright <newshound2 digitalmars.com> writes:
On 11/18/2013 1:04 PM, Jacob Carlborg wrote:

 On 2013-11-18 21:49, Walter Bright wrote:


 Most architectures I've used supported a HLT, and there's always
 something the back end could construct if it had to, even:

       ...
       call halt
       ...


 In that case, would we want it to be legal to throw AssertError instead of
using
 the halt instructions? Because that's what the docs says.


Sure.
Nov 18 2013
prev sibling parent reply "Jonathan M Davis" <jmdavisProg gmx.com> writes:
On Monday, November 18, 2013 22:04:59 Jacob Carlborg wrote:

 In that case, would we want it to be legal to throw AssertError instead
 of using the halt instructions? Because that's what the docs says.


For when you're not in release mode. You want the more informative AssertError 
in that case. HLT is just for when assertions get compiled out.

- Jonathan M Davis
Nov 18 2013
parent Jacob Carlborg <doob me.com> writes:
On 2013-11-18 23:21, Jonathan M Davis wrote:


 For when you're not in release mode. You want the more informative AssertError
 in that case. HLT is just for when assertions get compiled out.


I'm talking about release mode.

-- 
/Jacob Carlborg
Nov 18 2013
prev sibling parent "Dicebot" <public dicebot.lv> writes:
On Monday, 18 November 2013 at 20:49:48 UTC, Walter Bright wrote:

 Most architectures I've used supported a HLT, and there's 
 always something the back end could construct if it had to, 
 even:

      ...
      call halt
      ...


I am pretty sure on my x86_64 Linux `assert(0)` results in 
termination via SIGILL (Illegal Instruction). Well, it is still a 
hard crash so no big deal but probably in not an expected way.
Nov 19 2013
prev sibling parent reply Walter Bright <newshound2 digitalmars.com> writes:
On 11/18/2013 2:16 AM, Daniel Murphy wrote:

 Yeah.  On the other hand, if we decide assert(0) means 'assume unreachable'
 we can optimize out the try-catch in release mode, among other things.

 try { s } catch { assert(0); } -> s
 if (e) assert(0); else s; -> e; s;


I seriously object to this, as assert(0) is there for when you really do want
an 
assert in release builds.

I.e.:

     assert(!e);         // removed in release builds
     if (e) assert(0);   // stays in release builds

It's a valid D idiom, not a bug or oversight.
Nov 18 2013
parent reply "Jonathan M Davis" <jmdavisProg gmx.com> writes:
On Monday, November 18, 2013 12:47:32 Walter Bright wrote:

 On 11/18/2013 2:16 AM, Daniel Murphy wrote:

 Yeah. On the other hand, if we decide assert(0) means 'assume
 unreachable'
 we can optimize out the try-catch in release mode, among other things.
 
 try { s } catch { assert(0); } -> s
 if (e) assert(0); else s; -> e; s;


 
 I seriously object to this, as assert(0) is there for when you really do
 want an assert in release builds.
 
 I.e.:
 
 assert(!e); // removed in release builds
 if (e) assert(0); // stays in release builds
 
 It's a valid D idiom, not a bug or oversight.


Then what would you suggest for dealing with cases where a nothrow function 
calls a function which can throw in general but can't throw with the given 
arguments? Completely separate from the issue of whether std.datetime should 
need to be calling a throwing function from a nothrow function as often as it 
does, it's still an issue which must be dealt with in general. At present, any 
nothrow function which wants to call a throwing function needs to use a try-
catch block to wrap the throwing function, and it makes good sense to use 
assert(0) to catch the case where the caller screwed up and the function can 
indeed throw. However, given that that throwing function should never throw, 
it's desirable to be able to optimize out the try-catch-assert(0). How would 
we do that without taking advantage of the fact that assert(0) indicates that 
that line of code should be unreachable?

If this were  safe, we would use  trusted, but this is nothrow, and there is 
no trusted-nothrow.

- Jonathan M Davis
Nov 18 2013
parent reply Walter Bright <newshound2 digitalmars.com> writes:
On 11/18/2013 1:23 PM, Jonathan M Davis wrote:

 Then what would you suggest for dealing with cases where a nothrow function
 calls a function which can throw in general but can't throw with the given
 arguments? Completely separate from the issue of whether std.datetime should
 need to be calling a throwing function from a nothrow function as often as it
 does, it's still an issue which must be dealt with in general. At present, any
 nothrow function which wants to call a throwing function needs to use a try-
 catch block to wrap the throwing function, and it makes good sense to use
 assert(0) to catch the case where the caller screwed up and the function can
 indeed throw. However, given that that throwing function should never throw,
 it's desirable to be able to optimize out the try-catch-assert(0). How would
 we do that without taking advantage of the fact that assert(0) indicates that
 that line of code should be unreachable?


I believe this is solving the problem in the wrong place. The function being 
called should not be throwing, and should be defined as not throwing. See my 
other posts in this thread on the topic.
Nov 18 2013
next sibling parent reply "Jonathan M Davis" <jmdavisProg gmx.com> writes:
On Monday, November 18, 2013 13:50:42 Walter Bright wrote:

 On 11/18/2013 1:23 PM, Jonathan M Davis wrote:

 Then what would you suggest for dealing with cases where a nothrow
 function
 calls a function which can throw in general but can't throw with the given
 arguments? Completely separate from the issue of whether std.datetime
 should need to be calling a throwing function from a nothrow function as
 often as it does, it's still an issue which must be dealt with in
 general. At present, any nothrow function which wants to call a throwing
 function needs to use a try- catch block to wrap the throwing function,
 and it makes good sense to use assert(0) to catch the case where the
 caller screwed up and the function can indeed throw. However, given that
 that throwing function should never throw, it's desirable to be able to
 optimize out the try-catch-assert(0). How would we do that without taking
 advantage of the fact that assert(0) indicates that that line of code
 should be unreachable?


 
 I believe this is solving the problem in the wrong place. The function being
 called should not be throwing, and should be defined as not throwing. See
 my other posts in this thread on the topic.


Sometimes that's true, but if you're asserting that it's not going to be the 
case that folks are going to need to calling functions which can throw but 
won't throw from inside nothrow functions, I think that your dead wrong. If 
nothing else, you don't always have control over what is and isn't going to 
throw. Maybe it's uncommon enough that it's not worth trying to optimize out 
the try-catch blocks, but I think that asserting that you won't ever need to 
call a throwing function from a nothrow function when you know it won't throw 
is like asserting that  trusted isn't needed.

- Jonathan M Davis
Nov 18 2013
parent Walter Bright <newshound2 digitalmars.com> writes:
On 11/18/2013 2:16 PM, Jonathan M Davis wrote:

 Sometimes that's true, but if you're asserting that it's not going to be the
 case that folks are going to need to calling functions which can throw but
 won't throw from inside nothrow functions, I think that your dead wrong. If
 nothing else, you don't always have control over what is and isn't going to
 throw. Maybe it's uncommon enough that it's not worth trying to optimize out
 the try-catch blocks, but I think that asserting that you won't ever need to
 call a throwing function from a nothrow function when you know it won't throw
 is like asserting that  trusted isn't needed.


What I'm saying is it is bad API design to conflate data processing with input 
data validation. You may not always have control over the API design and may 
have to live with bad API design, but in std.datetime's case we do have control 
and we can do the right thing.

Joel Spolsky wrote a nice column 
http://www.joelonsoftware.com/articles/Wrong.html about this years ago, about 
conceptually separating input data validation operations from data crunching 
operations. This distinction is baked into D's design:

exceptions - input data validation

asserts - crunching on data that has been validated

Code like this:

    try {
         func();
    } catch (Exception e) {
        assert(0, "func() should not have thrown");
    }

clearly shows a design failure, either in func()'s design or how func() is used.
Nov 18 2013
prev sibling parent reply Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
On 11/18/13 1:50 PM, Walter Bright wrote:

 I believe this is solving the problem in the wrong place. The function
 being called should not be throwing, and should be defined as not
 throwing. See my other posts in this thread on the topic.


I think a stance of unceremoniously halting the program upon passing the 
wrong parameters to a standard library function is a bit excessive.

Andrei
Nov 18 2013
parent reply Walter Bright <newshound2 digitalmars.com> writes:
On 11/18/2013 2:37 PM, Andrei Alexandrescu wrote:

 On 11/18/13 1:50 PM, Walter Bright wrote:

 I believe this is solving the problem in the wrong place. The function
 being called should not be throwing, and should be defined as not
 throwing. See my other posts in this thread on the topic.


 I think a stance of unceremoniously halting the program upon passing the wrong
 parameters to a standard library function is a bit excessive.


Dang, I gotta argue this with you too? :-)

Forget the philosophical argument for the moment and let's try a pragmatic one
- 
code that doesn't check its arguments and doesn't throw is going to inline 
better, run faster, and be smaller. To have a high performance language, we
have 
to have this.

This is why we have a -release switch: so we can run with or without checking 
the arguments for bugs.

Input data validation is where exceptions come in. Those are relatively rare 
compared with processing validated data, and so separating out the slow data 
validation path from the validated high speed path makes pragmatic sense.

Rejecting this approach throws (!) out the whole concept of contract
programming.
Nov 18 2013
next sibling parent reply Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
On 11/18/13 2:49 PM, Walter Bright wrote:

 On 11/18/2013 2:37 PM, Andrei Alexandrescu wrote:

 On 11/18/13 1:50 PM, Walter Bright wrote:

 I believe this is solving the problem in the wrong place. The function
 being called should not be throwing, and should be defined as not
 throwing. See my other posts in this thread on the topic.


 I think a stance of unceremoniously halting the program upon passing
 the wrong
 parameters to a standard library function is a bit excessive.


 Dang, I gotta argue this with you too? :-)


Yes. I agree with your fundamental point but it is missing important 
nuances. I destroyed the misunderstanding in http://goo.gl/3xD5Aj.

Andrei
Nov 18 2013
parent Walter Bright <newshound2 digitalmars.com> writes:
On 11/18/2013 2:52 PM, Andrei Alexandrescu wrote:

 Yes. I agree with your fundamental point but it is missing important nuances. I
 destroyed the misunderstanding in http://goo.gl/3xD5Aj.


And I destroyed your destruction!

Anyhow, can you use real links rather than goo ones?
Nov 18 2013
prev sibling parent Dmitry Olshansky <dmitry.olsh gmail.com> writes:
19-Nov-2013 02:49, Walter Bright пишет:

 On 11/18/2013 2:37 PM, Andrei Alexandrescu wrote:

 On 11/18/13 1:50 PM, Walter Bright wrote:

 I believe this is solving the problem in the wrong place. The function
 being called should not be throwing, and should be defined as not
 throwing. See my other posts in this thread on the topic.


 I think a stance of unceremoniously halting the program upon passing
 the wrong
 parameters to a standard library function is a bit excessive.


 Dang, I gotta argue this with you too? :-)

 Forget the philosophical argument for the moment and let's try a
 pragmatic one - code that doesn't check its arguments and doesn't throw
 is going to inline better, run faster, and be smaller. To have a high
 performance language, we have to have this.

 This is why we have a -release switch: so we can run with or without
 checking the arguments for bugs.

 Input data validation is where exceptions come in. Those are relatively
 rare compared with processing validated data, and so separating out the
 slow data validation path from the validated high speed path makes
 pragmatic sense.

 Rejecting this approach throws (!) out the whole concept of contract
 programming.


There is a whole class of functions where validation is ~ the same 
amount of work as scrubbing the input. Do I need to provide an example 
of parser? :)

-- 
Dmitry Olshansky
Nov 19 2013
prev sibling parent reply Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
On 11/18/13 1:56 AM, Daniel Murphy wrote:

 "Andrei Alexandrescu" <SeeWebsiteForEmail erdani.org> wrote in message
 news:l6ccae$2cmt$1 digitalmars.com...

 1. Fix scope(failure) and then use it.


 scope(failure) picks up Errors as well as Exceptions


That's fine. What the compiler should do for a nothrow function goes as 
follows. Consider:

void fun() nothrow
{
    statementsA
    scope(failure) statement
    statementsB
}

First off, statementsA must not throw. Then, if statementsB may throw, 
then look at what statement throws. If statement always throws an Error, 
then accept the code. This is because the scope(failure) effectively 
translates whatever exception into an Error, and it's legal for a 
nothrow function to throw Error.

Currently it looks like the presence of scope(failure) simply makes the 
function seem legit, no matter what.

void fun() nothrow
{
    scope(failure) {}
    throw new Exception("so sue me");
}


Andrei
Nov 18 2013
parent reply "Daniel Murphy" <yebblies nospamgmail.com> writes:
"Andrei Alexandrescu" <SeeWebsiteForEmail erdani.org> wrote in message 
news:l6d9ki$ok6$1 digitalmars.com...

 First off, statementsA must not throw. Then, if statementsB may throw, 
 then look at what statement throws. If statement always throws an Error, 
 then accept the code. This is because the scope(failure) effectively 
 translates whatever exception into an Error, and it's legal for a nothrow 
 function to throw Error.


Yes.


 Currently it looks like the presence of scope(failure) simply makes the 
 function seem legit, no matter what.

 void fun() nothrow
 {
    scope(failure) {}
    throw new Exception("so sue me");
 }


Yes, obviously a bug.  The compiler is missing the rethrow.

My point was only that try-catch(Exception) and scope(failure) do not have 
the same semantics.
Nov 18 2013
next sibling parent Marco Leise <Marco.Leise gmx.de> writes:
Am Tue, 19 Nov 2013 04:00:56 +1100
schrieb "Daniel Murphy" <yebblies nospamgmail.com>:


 "Andrei Alexandrescu" <SeeWebsiteForEmail erdani.org> wrote in message 
 news:l6d9ki$ok6$1 digitalmars.com...

 Currently it looks like the presence of scope(failure) simply makes the 
 function seem legit, no matter what.

 void fun() nothrow
 {
    scope(failure) {}
    throw new Exception("so sue me");
 }


 
 Yes, obviously a bug.  The compiler is missing the rethrow.


That's severe. It's not even a rethrow, but an independent
throw statement sitting squarely in a nothrow function.

-- 
Marco
Nov 18 2013
prev sibling parent Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
On 11/18/13 9:00 AM, Daniel Murphy wrote:

 void fun() nothrow
 {
     scope(failure) {}
     throw new Exception("so sue me");
 }


 Yes, obviously a bug.  The compiler is missing the rethrow.


https://d.puremagic.com/issues/show_bug.cgi?id=11542


 My point was only that try-catch(Exception) and scope(failure) do not have
 the same semantics.


Now I got it, thanks. Far as I can tell in this case it doesn't make a 
big difference, is that correct?

a) with try/catch: Exceptions are transformed in AssertError, Errors 
just go through

b) with scope(failure): everything is transformed in AssertError.

There is a difference, but both behaviors would be acceptable.


Andrei
Nov 18 2013
prev sibling next sibling parent "Kagamin" <spam here.lot> writes:
As I understand, nothrow is used for minor optimization of 
exception handling, which doesn't help much if you hide mount of 
throwing code up the stack.
Nov 18 2013
prev sibling next sibling parent Walter Bright <newshound2 digitalmars.com> writes:
On 11/17/2013 10:32 PM, Andrei Alexandrescu wrote:

 4. ...?


Revisit why removeUnitsFromHNSecs, etc., are throwing. Since the caller is 
guaranteeing they don't throw, I suspect the runtime data validation is itself 
invalid and the throws should be replaced with asserts.
Nov 18 2013
prev sibling parent reply Shammah Chancellor <anonymous coward.com> writes:
On 2013-11-18 06:32:46 +0000, Andrei Alexandrescu said:

 
 1. Fix scope(failure) and then use it.
 
 Andrei


Huh?  Scope failure has no purpose here.  It does not CATCH the 
exception and prevent it from bubbling up the call chain.    Try/catch 
does do this.

-Shammah
Nov 21 2013
next sibling parent reply "growler" <growlercab gmail.com> writes:
On Friday, 22 November 2013 at 01:49:11 UTC, Shammah Chancellor 
wrote:

 On 2013-11-18 06:32:46 +0000, Andrei Alexandrescu said:

 
 1. Fix scope(failure) and then use it.
 
 Andrei


 Huh?  Scope failure has no purpose here.  It does not CATCH the 
 exception and prevent it from bubbling up the call chain.    
 Try/catch does do this.

 -Shammah


It does if you return from the scope(failure) block. The problem 
is you cannot mark the function as "nothrow"

For example:
---
void throwingFunction() {
     throw new Exception("damn!");
}

void someFunc() // nothrow
{
     scope(failure) {
         writeln("Failed in someFunc()");
         return;
     }
     throwingFunction();
}

void main() {
     try {
         someFunc();
         writeln("Yay, someFunc() is nothrow");
     } catch(Exception e) {
         writeln("An exception in main!");
     }
}

Output:

Failed in someFunc()
Yay, someFunc() is nothrow.
---

But you cannot mark someFunc() as nothrow.
Nov 21 2013
next sibling parent reply Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
On 11/21/13 6:07 PM, growler wrote:

 On Friday, 22 November 2013 at 01:49:11 UTC, Shammah Chancellor wrote:

 On 2013-11-18 06:32:46 +0000, Andrei Alexandrescu said:

 1. Fix scope(failure) and then use it.

 Andrei


 Huh?  Scope failure has no purpose here.  It does not CATCH the
 exception and prevent it from bubbling up the call chain. Try/catch
 does do this.

 -Shammah


 It does if you return from the scope(failure) block. The problem is you
 cannot mark the function as "nothrow"


I was thinking that scope(failure) could throw an Error.

Andrei
Nov 21 2013
parent "growler" <growlercab gmail.com> writes:
On Friday, 22 November 2013 at 02:17:18 UTC, Andrei Alexandrescu 
wrote:

 On 11/21/13 6:07 PM, growler wrote:

 On Friday, 22 November 2013 at 01:49:11 UTC, Shammah 
 Chancellor wrote:

 On 2013-11-18 06:32:46 +0000, Andrei Alexandrescu said:

 1. Fix scope(failure) and then use it.

 Andrei


 Huh?  Scope failure has no purpose here.  It does not CATCH 
 the
 exception and prevent it from bubbling up the call chain. 
 Try/catch
 does do this.

 -Shammah


 It does if you return from the scope(failure) block. The 
 problem is you
 cannot mark the function as "nothrow"


 I was thinking that scope(failure) could throw an Error.

 Andrei


Yes, throwing an error makes more sensee than just return;
Nov 21 2013
prev sibling parent reply Shammah Chancellor <anonymous coward.com> writes:
On 2013-11-22 02:07:36 +0000, growler said:


 On Friday, 22 November 2013 at 01:49:11 UTC, Shammah Chancellor wrote:

 On 2013-11-18 06:32:46 +0000, Andrei Alexandrescu said:

 
 1. Fix scope(failure) and then use it.
 
 Andrei


 
 Huh?  Scope failure has no purpose here.  It does not CATCH the 
 exception and prevent it from bubbling up the call chain.    Try/catch 
 does do this.
 
 -Shammah


 
 It does if you return from the scope(failure) block. The problem is you 
 cannot mark the function as "nothrow"
 
 For example:
 ---
 void throwingFunction() {
      throw new Exception("damn!");
 }
 
 void someFunc() // nothrow
 {
      scope(failure) {
          writeln("Failed in someFunc()");
          return;
      }
      throwingFunction();
 }
 
 void main() {
      try {
          someFunc();
          writeln("Yay, someFunc() is nothrow");
      } catch(Exception e) {
          writeln("An exception in main!");
      }
 }
 
 Output:
 
 Failed in someFunc()
 Yay, someFunc() is nothrow.
 ---
 
 But you cannot mark someFunc() as nothrow.


What!?  That shouldn't even be legal code!   See below for why:

void throwingFunction() {
     throw new Exception("damn!");
}

void someFunc() // nothrow
{
    scope(failure) { writeln("What?");} <-- NEVER EXECUTED?!
     scope(failure) {
         writeln("Failed in someFunc()");
         return;
     }
     throwingFunction();
}

void main() {
     try {
         someFunc();
         writeln("Yay, someFunc() is nothrow");
     } catch(Exception e) {
         writeln("An exception in main!");
     }
}
Nov 21 2013
next sibling parent reply "growler" <growlercab gmail.com> writes:
On Friday, 22 November 2013 at 02:35:40 UTC, Shammah Chancellor 
wrote:


...

 




 What!?  That shouldn't even be legal code!   See below for why:


...

Maybe that's why it is considered broken? The first point was to 
'fix' scope(failure) and then use it.

However, I don't know I haven't been following the discussion 
closely enough...
Nov 21 2013
parent Shammah Chancellor <anonymous coward.com> writes:
On 2013-11-22 02:51:11 +0000, growler said:


 Maybe that's why it is considered broken? The first point was to 'fix' 
 scope(failure) and then use it.
 
 However, I don't know I haven't been following the discussion closely enough...


I think fixing it means disallowing return and assert(0) here.  Since 
these things generate unreachable code.  If it's fixed in the way I 
think it should be, it still wouldn't be useful here.

The proposed fix was to make it so scope(failure) assert(0);  Is 
detected as preventing the exception, so functions could be marked 
nothrow.   I don't think that's a good idea for the above reason.

I'd rather see Walter's proposed fix 
(http://forum.dlang.org/thread/l6ccae$2cmt$1 digitalmars.com?page=3#post-l6ds2b:241ii3:24
:40digitalmars.com) 
of std.datetime, and my proposed fix for scope(failure).

-Shammah
Nov 22 2013
prev sibling parent reply "Jesse Phillips" <Jesse.K.Phillips+D gmail.com> writes:
On Friday, 22 November 2013 at 02:35:40 UTC, Shammah Chancellor 
wrote:

 void someFunc() // nothrow
 {
    scope(failure) { writeln("What?");} <-- NEVER EXECUTED?!
     scope(failure) {
         writeln("Failed in someFunc()");
         return;
     }
     throwingFunction();
 }


That does exactly as expected:

void someFunc()
{
     try {
         try {
             throwingFunction();
         } catch {
             writeln("Failed in someFunc()");
             return;
         }
     } catch {
         writeln("What?");
     }
}
Nov 21 2013
parent reply Shammah Chancellor <anonymous coward.com> writes:
On 2013-11-22 03:03:15 +0000, Jesse Phillips said:


 On Friday, 22 November 2013 at 02:35:40 UTC, Shammah Chancellor wrote:

 void someFunc() // nothrow
 {
 scope(failure) { writeln("What?");} <-- NEVER EXECUTED?!
 scope(failure) {
 writeln("Failed in someFunc()");
 return;
 }
 throwingFunction();
 }


 
 That does exactly as expected:
 
 void someFunc()
 {
      try {
          try {
              throwingFunction();
          } catch {
              writeln("Failed in someFunc()");
              return;
          }
      } catch {
          writeln("What?");
      }
 }


No.  scope(failure) is supposed to re-throw the exception.  Remember 
that the original proposal for scope(failure), etc. were using a stack 
of delegates.   The rewriting to try/catch is new and supposed to be 
functionally equivalent.  Return's from scope statements should be 
disallowed now.
Nov 22 2013
parent reply "growler" <growlercab gmail.com> writes:
On Friday, 22 November 2013 at 21:48:14 UTC, Shammah Chancellor 
wrote:

[snip]

 No.  scope(failure) is supposed to re-throw the exception.


Where does it say that?

http://dlang.org/statement.html#ScopeGuardStatement

"... scope(failure) executes NonEmptyOrScopeBlockStatement when 
the scope exits due to exception unwinding. ..."


[snip]

 Return's from scope statements should be disallowed now.


I disagree. You cannot return or throw from a scope(exit) or 
scope(success) for obvious reasons, but how a program behaves 
when a scope fails should left to the programmer.

I might be at a DLL or C/C++ boundary and any exceptions 
(Throwable or otherwise) I do not want to escape out of the 
current scope.

I know I could use try-catch, but I find scope is just much 
easier to read.

Cheers G.
Nov 22 2013
parent reply Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
On 11/22/13 5:37 PM, growler wrote:

 On Friday, 22 November 2013 at 21:48:14 UTC, Shammah Chancellor wrote:

 [snip]

 No.  scope(failure) is supposed to re-throw the exception.


 Where does it say that?

 http://dlang.org/statement.html#ScopeGuardStatement

 "... scope(failure) executes NonEmptyOrScopeBlockStatement when the
 scope exits due to exception unwinding. ..."


It's implied. The scope is already being exited by means of an 
exception. If scope(failure) changed anything, _that_ would have been 
documented.

Anyhow we can be clearer in the docs.


Andrei
Nov 22 2013
parent "growler" <growlercab gmail.com> writes:
On Saturday, 23 November 2013 at 01:48:28 UTC, Andrei 
Alexandrescu wrote:

 On 11/22/13 5:37 PM, growler wrote:

 On Friday, 22 November 2013 at 21:48:14 UTC, Shammah 
 Chancellor wrote:

 [snip]

 No.  scope(failure) is supposed to re-throw the exception.


 Where does it say that?

 http://dlang.org/statement.html#ScopeGuardStatement

 "... scope(failure) executes NonEmptyOrScopeBlockStatement 
 when the
 scope exits due to exception unwinding. ..."


 It's implied. The scope is already being exited by means of an 
 exception. If scope(failure) changed anything, _that_ would 
 have been documented.

 Anyhow we can be clearer in the docs.


 Andrei


I stand corrected, cheers.
Nov 22 2013
prev sibling parent reply "Jesse Phillips" <Jesse.K.Phillips+D gmail.com> writes:
On Friday, 22 November 2013 at 01:49:11 UTC, Shammah Chancellor 
wrote:

 On 2013-11-18 06:32:46 +0000, Andrei Alexandrescu said:

 
 1. Fix scope(failure) and then use it.
 
 Andrei


 Huh?  Scope failure has no purpose here.  It does not CATCH the 
 exception and prevent it from bubbling up the call chain.    
 Try/catch does do this.

 -Shammah


If the code is:

     scope(failure) assert(0);

Then it is a statement that the function doesn't throw. So there 
isn't a need to catch the exception, it is merely a way to state 
to the compiler "I've verified I know what I'm talking about and 
this function really doesn't ever throw."

Right now just having scope(failure) in the body is making this 
statement, and that is wrong.
Nov 21 2013
parent Shammah Chancellor <anonymous coward.com> writes:
On 2013-11-22 02:13:42 +0000, Jesse Phillips said:


 On Friday, 22 November 2013 at 01:49:11 UTC, Shammah Chancellor wrote:

 On 2013-11-18 06:32:46 +0000, Andrei Alexandrescu said:

 
 1. Fix scope(failure) and then use it.
 
 Andrei


 
 Huh?  Scope failure has no purpose here.  It does not CATCH the 
 exception and prevent it from bubbling up the call chain.    Try/catch 
 does do this.
 
 -Shammah


 
 If the code is:
 
      scope(failure) assert(0);
 
 Then it is a statement that the function doesn't throw. So there isn't 
 a need to catch the exception, it is merely a way to state to the 
 compiler "I've verified I know what I'm talking about and this function 
 really doesn't ever throw."
 
 Right now just having scope(failure) in the body is making this 
 statement, and that is wrong.


Scope failure blocks should not be able to raise errors, or return 
except "out the end".
Nov 22 2013