Skip to content
This repository

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse code

Add GetOptException to std.getopt to avoid throwing plain Exception.

  • Loading branch information...
commit 4b87dcf39efeb4ddafe8fe99a0ef9a529c0dcaca 1 parent 786284e
authored February 18, 2012

Showing 1 changed file with 18 additions and 4 deletions. Show diff stats Hide diff stats

  1. 22  std/getopt.d
22  std/getopt.d
@@ -79,7 +79,8 @@ void main(string[] args)
79 79
  to their defaults and then invoke $(D getopt). If a
80 80
  command-line argument is recognized as an option with a parameter and
81 81
  the parameter cannot be parsed properly (e.g. a number is expected
82  
- but not present), a $(D ConvException) exception is thrown.
  82
+ but not present), a $(D ConvException) exception is thrown. On all
  83
+ other errors, a $(D GetOptException) exception is thrown.
83 84
 
84 85
  Depending on the type of the pointer being bound, $(D getopt)
85 86
  recognizes the following kinds of options:
@@ -346,6 +347,18 @@ void getopt(T...)(ref string[] args, T opts) {
346 347
 }
347 348
 
348 349
 /**
  350
+ * Thrown if an error occurs while parsing the command line.
  351
+ */
  352
+
  353
+class GetOptException : Exception
  354
+{
  355
+    this(string s, string fn = __FILE__, size_t ln = __LINE__)
  356
+    {
  357
+        super(s, fn, ln);
  358
+    }
  359
+}
  360
+
  361
+/**
349 362
  * Configuration options for $(D getopt). You can pass them to $(D
350 363
  * getopt) in any position, except in between an option string and its
351 364
  * bound pointer.
@@ -408,7 +421,7 @@ private void getoptImpl(T...)(ref string[] args,
408 421
             if (endOfOptions.length && a == endOfOptions) break;
409 422
             if (!cfg.passThrough)
410 423
             {
411  
-                throw new Exception("Unrecognized option "~a);
  424
+                throw new GetOptException("Unrecognized option " ~ a ~ ".");
412 425
             }
413 426
         }
414 427
     }
@@ -464,8 +477,9 @@ void handleOption(R)(string option, R receiver, ref string[] args,
464 477
             if (!isDelegateWithLessThanTwoParameters && !(val.length) && !incremental) {
465 478
                 // Eat the next argument too.  Check to make sure there's one
466 479
                 // to be eaten first, though.
467  
-                enforce(i < args.length,
468  
-                    "Missing value for argument " ~ a ~ ".");
  480
+                if (i >= args.length)
  481
+                    throw new GetOptException("Missing value for argument " ~ a ~ ".");
  482
+
469 483
                 val = args[i];
470 484
                 args = args[0 .. i] ~ args[i + 1 .. $];
471 485
             }

5 notes on commit 4b87dcf

Andrei Alexandrescu

I'll prefer to hold off on this. I generally disagree with the notion there should be many exception types (and consequently one per module). A better solution is to include the module name in the exception fields. Anyhow, we must carefully review our general doctrine on exceptions.

Alex Rønne Petersen
Owner

The entire point of specific exception types is to not erroneously catch exceptions you did not intend to catch. It's not about per-module exceptions, it's simply about correctness of catch blocks. Throwing plain Exception encourages swallowing errors you did not intend to.

Encoding the module name in the exception is not sufficient. That forces me to check some field of the caught exception rather than just catching a specific type of exception. This approach would also mean that I have to rethrow the exception if I do not wish to handle it. That's just annoying... and also resets the stack trace, making debugging a nightmare.

Andrei Alexandrescu

The converse point is that the whole purpose of exceptions is to enable centralized error handling, and distinguishing them by type encourages the exact opposite.

More to the point, in most C++ applications I worked with, there was no meaningful use of distinct exception types.

Andrei Alexandrescu

Let's take this to the newsgroup and start a flame war. I'm sure something good will come out of it. For my money, I don't know what the right way is, but I feel yours isn't.

Alex Rønne Petersen
Owner

Yes, I did notice this trend back when I worked with C++.

However, if you look at most C# or Java applications, it's the complete opposite: People try to catch specific exceptions such that exceptions that indicate underlying problems can surface and be noticed, rather than silenced and potentially ignored. Catching everything is good in some applications, because reliability (i.e. don't crash on an unhandled exception) is very important. In others, correctness is MUCH more important (compilers, virtual machines, that sort of thing). In those cases, you want to know as soon as possible when something is wrong so you can fix it.

Now, that argument doesn't really hold water in the case of std.getopt of all things, but I do think that we need a consistent design across all of Phobos.

Please sign in to comment.
Something went wrong with that request. Please try again.