[Liblas-commits] hg-main-tree: use exceptions, not status codes, for simpler cont...

liblas-commits at liblas.org liblas-commits at liblas.org
Tue Aug 16 15:13:11 EDT 2011


details:   http://hg.libpc.orghg-main-tree/rev/46e987465d22
changeset: 1113:46e987465d22
user:      Michael P. Gerlek <mpg at flaxen.com>
date:      Tue Aug 16 12:12:56 2011 -0700
description:
use exceptions, not status codes, for simpler control flow

diffstat:

 apps/Application.cpp |  75 +++++++++++++++++++++++++++++----------------------
 apps/Application.hpp |  30 +++++++++++++++++---
 apps/pc2pc.cpp       |  15 ++++------
 apps/pcinfo.cpp      |  19 +++++-------
 apps/pcpipeline.cpp  |  12 +++----
 5 files changed, 87 insertions(+), 64 deletions(-)

diffs (truncated from 336 to 300 lines):

diff -r 2739a27fb75e -r 46e987465d22 apps/Application.cpp
--- a/apps/Application.cpp	Tue Aug 16 11:47:54 2011 -0700
+++ b/apps/Application.cpp	Tue Aug 16 12:12:56 2011 -0700
@@ -54,8 +54,40 @@
     return;
 }
 
+
+// this just wraps ALL the code in total catch block
 int Application::run()
 {
+    int status = 1;
+
+    try
+    {
+        status = innerRun();
+    }
+    catch (pdal::pdal_error e)
+    {
+        const std::string s("Caught PDAL exception: ");
+        printError(s + e.what());
+        status = 1;
+    }
+    catch (std::exception e)
+    {
+        const std::string s("Caught exception: ");
+        printError(s + e.what());
+        status = 1;
+    }
+    catch (...)
+    {
+        printError("Caught unknown exception");
+        status = 1;
+    }
+
+    return status;
+}
+
+
+int Application::innerRun()
+{
     // add -h, -v, etc
     addBasicOptionSet();
 
@@ -78,35 +110,22 @@
         return 0;
     }
 
-    // do any user-level sanity checking
-    bool happy = validateOptions();
-    if (!happy)
+    try
     {
+        // do any user-level sanity checking
+        validateOptions();
+    }
+    catch (app_usage_error e)
+    {
+        printError(e.what());
         outputHelp();
         return 1;
     }
 
     boost::timer timer;
     
-    // call derived function
-    int status = 0;
+    int status = execute();
     
-    try
-    {
-        status = execute();
-    }
-    catch (std::exception e)
-    {
-        const std::string s(e.what());
-        runtimeError("Caught exception: " + s);
-        status = 1;
-    }
-    catch (...)
-    {
-        runtimeError("Caught unknown exception");
-        status = 1;
-    }
-
     if (status == 0 && hasOption("timer"))
     {
         const double t = timer.elapsed();
@@ -117,16 +136,9 @@
 }
 
 
-void Application::usageError(const std::string& err)
+void Application::printError(const std::string& err)
 {
-    std::cout << "Usage error: " << err << std::endl;
-    std::cout << std::endl;
-}
-
-
-void Application::runtimeError(const std::string& err)
-{
-    std::cout << "Error: " << err << std::endl;
+    std::cout << err << std::endl;
     std::cout << std::endl;
 }
 
@@ -234,8 +246,7 @@
     }
     catch (boost::program_options::unknown_option e)
     {
-        usageError("unknown option: " + e.get_option_name());
-        exit(1);
+        throw app_usage_error("unknown option: " + e.get_option_name());
     }
 
     po::notify(m_variablesMap);
diff -r 2739a27fb75e -r 46e987465d22 apps/Application.hpp
--- a/apps/Application.hpp	Tue Aug 16 11:47:54 2011 -0700
+++ b/apps/Application.hpp	Tue Aug 16 12:12:56 2011 -0700
@@ -35,6 +35,7 @@
 #ifndef INCLUDED_APPLICATION_HPP
 #define INCLUDED_APPLICATION_HPP
 
+#include <pdal/exceptions.hpp>
 #include <iosfwd>
 
 
@@ -48,8 +49,26 @@
 #endif
 
 
+class app_usage_error : public pdal::pdal_error
+{
+public:
+    app_usage_error(std::string const& msg)
+        : pdal_error(msg)
+    {}
+};
+
+
+class app_runtime_error : public pdal::pdal_error
+{
+public:
+    app_runtime_error(std::string const& msg)
+        : pdal_error(msg)
+    {}
+};
+
+
 //
-// The pplication base class gives us these common options:
+// The application base class gives us these common options:
 //    --help / -h
 //    --verbose / -v
 //    --version
@@ -66,10 +85,11 @@
     virtual void addOptions() = 0;
 
     // implement this, to do sanity checking of cmd line
-    // return false if the user gave us bad options
-    virtual bool validateOptions() { return true; }
+    // will throw if the user gave us bad options
+    virtual void validateOptions() {}
 
     // implement this, to do your actual work
+    // it will be wrapped in a global catch try/block for you
     virtual int execute() = 0;
 
 protected:
@@ -79,10 +99,10 @@
     bool isDebug() const;
     boost::uint8_t getVerboseLevel() const;
     bool hasOption(const std::string& name);
-    void usageError(const std::string&);
-    void runtimeError(const std::string&);
+    void printError(const std::string&);
 
 private:
+    int innerRun();
     void parseOptions();
     void outputHelp();
     void outputVersion();
diff -r 2739a27fb75e -r 46e987465d22 apps/pc2pc.cpp
--- a/apps/pc2pc.cpp	Tue Aug 16 11:47:54 2011 -0700
+++ b/apps/pc2pc.cpp	Tue Aug 16 12:12:56 2011 -0700
@@ -62,7 +62,7 @@
 
 private:
     void addOptions();
-    bool validateOptions();
+    void validateOptions();
 
     std::string m_inputFile;
     std::string m_outputFile;
@@ -79,21 +79,19 @@
 }
 
 
-bool Application_pc2pc::validateOptions()
+void Application_pc2pc::validateOptions()
 {
     if (!hasOption("input"))
     {
-        usageError("--input/-i required");
-        return false;
+        throw app_usage_error("--input/-i required");
     }
 
     if (!hasOption("output"))
     {
-        usageError("--output/-o required");
-        return false;
+        throw app_usage_error("--output/-o required");
     }
 
-    return true;
+    return;
 }
 
 
@@ -118,8 +116,7 @@
 {
     if (!FileUtils::fileExists(m_inputFile))
     {
-        runtimeError("file not found: " + m_inputFile);
-        return 1;
+        throw app_runtime_error("file not found: " + m_inputFile);
     }
 
     std::ostream* ofs = FileUtils::createFile(m_outputFile);
diff -r 2739a27fb75e -r 46e987465d22 apps/pcinfo.cpp
--- a/apps/pcinfo.cpp	Tue Aug 16 11:47:54 2011 -0700
+++ b/apps/pcinfo.cpp	Tue Aug 16 12:12:56 2011 -0700
@@ -56,7 +56,7 @@
 
 private:
     void addOptions();
-    bool validateOptions();
+    void validateOptions();
     void readOnePoint();
     void readAllPoints();
 
@@ -73,18 +73,18 @@
     , m_inputFile("")
     , m_pointNumber(0)
 {
+    return;
 }
 
 
-bool PcInfo::validateOptions()
+void PcInfo::validateOptions()
 {
     if (!hasOption("input"))
     {
-        usageError("input file name required");
-        return false;
+        throw app_usage_error("input file name required");
     }
 
-    return true;
+    return;
 }
 
 
@@ -129,8 +129,7 @@
     const boost::uint32_t numRead = iter->read(data);
     if (numRead != 1)
     {
-        runtimeError("problem reading point number " + m_pointNumber);
-        return;
+        throw app_runtime_error("problem reading point number " + m_pointNumber);
     }
 
     std::cout << "Read point " << m_pointNumber << "\n";
@@ -165,16 +164,14 @@
 {
     if (!pdal::FileUtils::fileExists(m_inputFile))
     {
-        runtimeError("file not found: " + m_inputFile);
-        return 1;
+        throw app_runtime_error("file not found: " + m_inputFile);
     }
 
     std::string driver = AppSupport::inferReaderDriver(m_inputFile);
 
     if (driver == "")
     {
-        runtimeError("Cannot determine file type of " + m_inputFile);
-        return 1;
+        throw app_runtime_error("Cannot determine file type of " + m_inputFile);
     }
 
     if (hasOption("liblas") && driver == "drivers.las.reader")
diff -r 2739a27fb75e -r 46e987465d22 apps/pcpipeline.cpp
--- a/apps/pcpipeline.cpp	Tue Aug 16 11:47:54 2011 -0700
+++ b/apps/pcpipeline.cpp	Tue Aug 16 12:12:56 2011 -0700
@@ -54,7 +54,7 @@
 


More information about the Liblas-commits mailing list