On Sunday, February 19, 2012 00:21:38 Robert Jacques wrote:
> Not to jump on you in particular :) but bad user parameters should never be treated as exceptional. Even bad 'internal' parameters that are passed via the external API aren't exceptional. Programmers being lazy about input parameter checking is how hackers make their money.

I completely disagree. I think that there are many cases where throwing an exception on bad parameters is exactly what you should do. Phobos does this in many places. Often, the function knows best what it wants, and it's the best position to check it. So, having it check the input makes a _lot_ of sense in many cases.

For instance, take fromISOExtString on std.datetime.SysTime. Is it at all reasonably to expect that the caller is going to verify that the string is well-formed according to the ISO standard? No. And it's highly probable tha the string being given came from I/O of some kind - be it a file or a socket or whatever. So, it makes _way_ more sense for fromISOExtString to check the string itself and throw on failure. It's also more efficient to do that, because if you checked that the string was valid before passing it to fromISOExtString, then you'd be processing the string twice.

There are other cases where you want to use DbC and require that a function is given valid arguments. In those cases, you use assertions, so the checks go away in release mode. But that's not always the best way to go about. And in defensive programming, you end up checking exceptions quite heavily - even in release mode - and throwing when they're bad.

The best approach depends on what you're doing, and what the functions are used for. But approaches have their place.

- Jonathan M Davis

On Sunday, February 19, 2012 00:43:58 Andrei Alexandrescu wrote:
> On 2/18/12 8:00 PM, H. S. Teoh wrote:
> >>  From this and other posts I'd say we need to design the base exception
> >> 
> >> classes better, for example by defining an overridable property isTransient that tells caller code whether retrying might help.
> > 
> > Just because an exception is transient doesn't mean it makes sense to try again. For example, saveFileMenu() might read a filename from the user, then save the data to a file. If the user types an invalid filename, you will get an InvalidFilename exception. From an abstract point of view, an invalid filename is not a transient problem: retrying the invalid filename won't make the problem go away. But the application in this case *wants* to repeat the operation by asking the user for a *different* filename.
> > 
> > On the other hand, if the same exception happens in an app that's trying to read a configuration file, then it *shouldn't* retry the operation.
> 
> I'm thinking an error is transient if retrying the operation with the same exact data may succeed. That's a definition that's simple, useful, and easy to operate with.

A core problem with the idea is that whether or not it makes sense to try again depends on what the caller is doing. In general, I think that it's best to give the caller as much useful information is possible so that _it_ can decide the best way to handle the exception.

- Jonathan M Davis

"Jonathan M Davis" <jmdavisProg@gmx.com> wrote in message news:mailman.582.1329634661.20196.digitalmars-d@puremagic.com...
> On Sunday, February 19, 2012 16:59:49 Daniel Murphy wrote:
>> "Nick Sabalausky" <a@a.a> wrote in message news:jhprac$2aj$1@digitalmars.com...
>>
>> > The only problem I've ever had with them is that there's no templated
>> > catch blocks, so you can't handle two different exceptions with the
>> > same
>> > code without violating DRY or worse: catching the common base type and
>> > rethrowing when it's not what you wanted. Toss in templated catch
>> > blocks,
>> > and I've have no problem at all.
>>
>> Do you mean something like this?
>> try
>> {
>>     something();
>> }
>> catch (e : ThisException, ThatException, OtherException)
>> {
>>     static assert(is(typeof(e) == CommonType!(ThisException,
>> ThatException,
>> OtherException));
>> }
>> catch (Exception e)
>> {
>>     // Every other type derived from Exception
>> }
>>
>> Or do you think the full power to be able to template catch blocks as if they were functions would be useful for something?
>
> I think that being able to have a catch block which took multiple
> exception
> types would be plenty. There are times when it would be very valuable to
> be
> able to use the same catch block for multiple exceptions without having to
> catch their base type (which would then potentially catch other exceptions
> which you didn't want to catch). So, something like that second catch
> block
> that you have there would be very valuable.
>
> - Jonathan M Davis

I assume you mean the _first_ catch block?

By the looks of it java 7 has this with an awful syntax.

Maybe we should introduce implicit 'catch fallthrough':

void fun()
{
    try { something; }
    catch (ExceptionA) {} // falls through implicitly
    catch (ExpcetionB)
    { // handle both A and B
        break;
    }
    catch (ExceptionC)
    {
        // handle exception C
        break;
    }
}

On Sunday, February 19, 2012 18:44:30 Daniel Murphy wrote:
> I assume you mean the _first_ catch block?

Yes. I should have been more specific.

> By the looks of it java 7 has this with an awful syntax.
> 
> Maybe we should introduce implicit 'catch fallthrough':
> 
> void fun()
> {
>     try { something; }
>     catch (ExceptionA) {} // falls through implicitly
>     catch (ExpcetionB)
>     { // handle both A and B
>         break;
>     }
>     catch (ExceptionC)
>     {
>         // handle exception C
>         break;
>     }
> }

That wouldn't work, because it would make it so that ignore an exception in the middle if you wanted to (much as that's generally bad practice). It would also have the problem that you couldn't access the exception itself, and while you obviously wouldn't be operating on the exception's exact type regardless, you might want to iteract with the functions on a common based class.

So, while at first glance, it seems like a good idea, I think that it has too many issues as-is to work. It might be possible to adjust the idea to make it workable though. Right now, it's possible to do it via mixins or calling a function inside the catch, but doing something similar to this would certainly be nice, assuming that we could sort out the kinks.

- Jonathan M Davis

"Jonathan M Davis" <jmdavisProg@gmx.com> wrote in message news:mailman.585.1329637995.20196.digitalmars-d@puremagic.com...
> On Sunday, February 19, 2012 18:44:30 Daniel Murphy wrote:
>> I assume you mean the _first_ catch block?
>
> Yes. I should have been more specific.
>
>> By the looks of it java 7 has this with an awful syntax.
>>
>> Maybe we should introduce implicit 'catch fallthrough':
>>
>> void fun()
>> {
>>     try { something; }
>>     catch (ExceptionA) {} // falls through implicitly
>>     catch (ExpcetionB)
>>     { // handle both A and B
>>         break;
>>     }
>>     catch (ExceptionC)
>>     {
>>         // handle exception C
>>         break;
>>     }
>> }
>
> That wouldn't work, because it would make it so that ignore an exception
> in
> the middle if you wanted to (much as that's generally bad practice). It
> would
> also have the problem that you couldn't access the exception itself, and
> while
> you obviously wouldn't be operating on the exception's exact type
> regardless,
> you might want to iteract with the functions on a common based class.
>
> So, while at first glance, it seems like a good idea, I think that it has
> too
> many issues as-is to work. It might be possible to adjust the idea to make
> it
> workable though. Right now, it's possible to do it via mixins or calling a
> function inside the catch, but doing something similar to this would
> certainly
> be nice, assuming that we could sort out the kinks.
>
> - Jonathan M Davis

I wasn't really serious about implicit fallthrough.

Out of the syntaxes I could come up with:
catch(Ex1, Ex2 e)
catch(e : Ex1, Ex2)
catch(Ex1 | Ex2 e) // java 7 syntax, horrible

I like (e : list) the best.  Naturally it would also accept a type tuple of exceptions.

http://d.puremagic.com/issues/show_bug.cgi?id=7540

On 2/19/12 12:54 AM, H. S. Teoh wrote:
> On Sun, Feb 19, 2012 at 12:43:58AM -0600, Andrei Alexandrescu wrote:
>> On 2/18/12 8:00 PM, H. S. Teoh wrote:
>>>>  From this and other posts I'd say we need to design the base exception
>>>> classes better, for example by defining an overridable property
>>>> isTransient that tells caller code whether retrying might help.
>>>
>>> Just because an exception is transient doesn't mean it makes sense to
>>> try again. For example, saveFileMenu() might read a filename from the
>>> user, then save the data to a file. If the user types an invalid
>>> filename, you will get an InvalidFilename exception. From an abstract
>>> point of view, an invalid filename is not a transient problem: retrying
>>> the invalid filename won't make the problem go away. But the application
>>> in this case *wants* to repeat the operation by asking the user for a
>>> *different* filename.
>>>
>>> On the other hand, if the same exception happens in an app that's trying
>>> to read a configuration file, then it *shouldn't* retry the operation.
>>
>> I'm thinking an error is transient if retrying the operation with the
>> same exact data may succeed. That's a definition that's simple,
>> useful, and easy to operate with.
> [...]
>
> But if that's the case, what's the use of an exception at all?

Centralization.

Andrei

On 2/19/12 12:56 AM, Jonathan M Davis wrote:
> I think that being able to have a catch block which took multiple exception
> types would be plenty. There are times when it would be very valuable to be
> able to use the same catch block for multiple exceptions without having to
> catch their base type (which would then potentially catch other exceptions
> which you didn't want to catch). So, something like that second catch block
> that you have there would be very valuable.

So now hierarchies are not good?

Andrei

On 2/19/12 1:12 AM, Jonathan M Davis wrote:
> On Sunday, February 19, 2012 00:43:58 Andrei Alexandrescu wrote:
>> On 2/18/12 8:00 PM, H. S. Teoh wrote:
>>>>    From this and other posts I'd say we need to design the base exception
>>>>
>>>> classes better, for example by defining an overridable property
>>>> isTransient that tells caller code whether retrying might help.
>>>
>>> Just because an exception is transient doesn't mean it makes sense to
>>> try again. For example, saveFileMenu() might read a filename from the
>>> user, then save the data to a file. If the user types an invalid
>>> filename, you will get an InvalidFilename exception. From an abstract
>>> point of view, an invalid filename is not a transient problem: retrying
>>> the invalid filename won't make the problem go away. But the application
>>> in this case *wants* to repeat the operation by asking the user for a
>>> *different* filename.
>>>
>>> On the other hand, if the same exception happens in an app that's trying
>>> to read a configuration file, then it *shouldn't* retry the operation.
>>
>> I'm thinking an error is transient if retrying the operation with the
>> same exact data may succeed. That's a definition that's simple, useful,
>> and easy to operate with.
>
> A core problem with the idea is that whether or not it makes sense to try
> again depends on what the caller is doing. In general, I think that it's best
> to give the caller as much useful information is possible so that _it_ can
> decide the best way to handle the exception.

That sounds like "I violently agree".

Andrei

On Sun, Feb 19, 2012 at 01:05:25AM -0600, Andrei Alexandrescu wrote: [...]
> Ideally we'd have the right number of types - not more, not less.  The main purpose of this thread is to figure how many exception types are "just right". There has been a tendency in Phobos to just add a module-specific exception to certain modules, without a good reason. I'm trying to figure out the reasons.

As a first stab at some reasons, what about something along these lines:

- Would the *user* of the module care to catch that particular (group
  of) exception(s) as opposed to just catching all exceptions in
  general?  Does that exception give semantically valuable information
  that the user might want to act on, or it is just programmer
  convenience (it throws X just because it belongs to module Y)?

- Does it add domain-specific error information that is useful (errno,
  Oracle error codes, etc.)?


> >In the case of getopt, at _least_ adding a GetOptException with a field for the failed flag would be very valuable, and having additional derived types which indicate _why_ it failed would also be valuable.
> 
> The additional state sounds fine, but I'm not so sure about adding one type per failure reason.

When to stop introducing leaf nodes in the hierarchy is something worth thinking about in more depth. There are some pros and cons to consider.

To make the discussion grounded in reality, let's consider the example
of file I/O. We could either (1) introduce a base class, say
IOException, along with as many subclasses as necessary to capture all
possible I/O errors (with possibly multiple layers of exceptions); or
(2) introduce a base class IOException with no subclasses, but some
attributes like errno or something equivalent, that describes the error.
Or (3), some compromise between (1) and (2).

For (1):

- Pros:
   - You can be as general or as specific as needed. If you want to
     catch a specific error, you can. If you want to catch all I/O
     exceptions, you can.
   - Encapsulates OS and platform-specific errors in generic cross
     platform types that can be handled generically. (I.e., you don't
     have to know that std.io uses errno in interacting with the OS, and
     you don't have to know what errno values are. A generic
     FileNotFoundException properly maps to whatever OS you compile on.)
- Cons:
   - Too many tiny classes, many differing only in type with no real
     additional information beyond the type itself.
   - Does not necessarily capture everything the user may want to catch.
     Say under IOException you have FileException and SocketException,
     with more classes underneath. If you want to catch a subset of
     classes directly under FileException, there's no clean way to do so
     unless we expand the type-matching in catch() blocks.
   - May require a lot of effort to implement but only with diminishing
     returns (most programs won't bother with the distinction between
     two finely-differentiated exceptions).

For (2):

- Pros:
   - No class bloat.
   - Don't have to wade through reams of documentation to know what to
     catch. Just catch IOException and decide what to do based on the
     data encoded (say, errno value, or equivalent).
- Cons:
   - Risks exposing OS-specific and implementation-specific details to
     caller unnecessarily. E.g., if you use errno, then caller is tied
     to C's stdio. Then you can't change the underlying implementation
     to, say, iostream, or something else altogether, without massive
     breakage.
   - To only catch a specific error, you'd have to catch IOException,
     check the error code, and rethrow if it isn't what you want. Ugly.
     (It's like saying, oh, I can handle all IOExceptions, oh here's an
     IOException! Er, nevermind, I was kidding, I don't handle this
     particular IOException after all.)

I don't have the answers. Just want to put the pros and cons down so that we have something concrete to debate about.


> >And in some cases at least, that would lead to more member fields in the derived exceptions (like the value of the flag if it were given a bad value).
> 
> And aside from that? The universe of possible errors in getopt is finite, closed, and actually fairly small. Can't we do with fewer types?

Getopt isn't really the best use case for elaborate exception hierarchies. Perhaps we should use file I/O or network/socket I/O instead.


[...]
> How about a system in which you can say whether an exception is I/O related, network related, recoverable or not, should be displayed to the user or not, etc. Such is difficult to represent with inheritance alone.

The problem is, the thrower can't make the decision of whether or not something should be displayed to the user. You simply don't have the information nor context to decide that. That is completely dependent on what the calling app is trying to accomplish.

A disk diagnostic program will want to display everything to the user, all the full gory details.  A 3D shooter just wants to say "yo, something's wrong with da filesystem, I be quittin'!" and abort. Displaying some obscure error about some obscure problem with some obscure data file is worthless to the player.  A database server backend has no *way* to display any error at all, just log it to some logfile. And server logfiles generally want to record ALL exceptions, not just some subset arbitrarily assigned by Phobos developers.

Same thing with whether something is recoverable. The most horrendous I/O errors are "recoverable" from the POV of a disk repair program. The same errors should cause a word-processing app to die horribly instead of blindly charging forward and causing more data loss. You simply have no way of deciding this at the library level.

As for marking whether an exception is network-related: you're just bloating the base class with mostly-irrelevant information. Why does every file I/O exception have to indicate they are not network errors? Why does every parser error have to say they are not network errors? Lexer errors? Regex errors? Database file corruption errors?

The whole point of using a *class hierarchy* is so that all network errors derive from NetworkException, which in turn derives from Exception. The value of is_network_related is automatically, cleanly, and logically encapsulated by the act of defining a subclass of Exception. You don't need to explicitly represent it. It's implied by the existence of the subtype (which translates to a kind of representation in the form of the associated Typeinfo, if you'd like to think of it that way).


[...]
> I think right now we're erring a bit on the side of defining useless and uninformative exception types. As far as the originating module goes, it makes perfect sense to make that a field in the base class.

But "useless" and "uninformative" is relative to what the *caller* wants to achieve. The difference between FileNotFound and ReadError is irrelevant to a program that's trying to load startup configuration files. But a program that's trying to decide whether or not to prompt the user for a different filename needs to know this distinction. The *same* program won't care for this distinction when *it* is trying to load configuration files.

Basically, there's no sane way you can decide this for the calling app at the library level.


> Anyway, a simple action items right now is improve Exception's offering with things like e.g. custom formatting for i18n, more origin information, cloning, attributes and capabilities (transitory, user-visible etc).
[...]

But putting too much into the Exception base class risks having a whole bunch of fields that nobody uses. Why should you need to specify 10 different attributes, most of which you don't care about, every time you throw an exception?  Why not only introduce additional fields as necessary?  I.e., use a class hierarchy! That's what a class hierarchy is for, to keep the base class providing only *basic* functionality, and derived classes to add additional info for those subsets of exceptions that need that info.

Again, I'm not saying that an elaborated exception hierarchy is the be all and end all of exception handling. It's just that I have yet to see a better system that provides those attributes I listed in another reply: the ability to be as general/specific as needed, and a way to programmatically recover from an exception based on information provided by the exception object (which entails domain-specific errors encoded in a sane way).

If you can come up with a better system, by all means, please do. I'm listening.


T

-- 
If you look at a thing nine hundred and ninety-nine times, you are perfectly safe; if you look at it the thousandth time, you are in frightful danger of seeing it for the first time. -- G. K. Chesterton

On Sat, Feb 18, 2012 at 11:52:00PM -0800, Jonathan M Davis wrote: [...]
> So, while at first glance, it seems like a good idea, I think that it has too many issues as-is to work. It might be possible to adjust the idea to make it workable though. Right now, it's possible to do it via mixins or calling a function inside the catch, but doing something similar to this would certainly be nice, assuming that we could sort out the kinks.
[...]

I have an idea. What about "signature constraints" for catch, ala template signature constraints? Something like this:

	try {
		...
	} catch(IOException e)
		if (e.errno in subsetYouWantToHandle)
	{
		...
	}

Just using IOException as an example. The idea is to allow arbitrary expressions in the constraint so whatever doesn't satisfy the constraint will be regarded as "not caught", even if the base type matches.


T

-- 
Gone Chopin. Bach in a minuet.