The SuccessException



  • While spelunking around in a section of our system I've not previously encountered, I discovered that our offshore developers like doing error checking in the old "C" way:

       int rc = someFunc();
    if (rc != 0) {
    return rc; // error
    }

    ...and then checking for it everywhere. We begged, pleaded, insisted, and finally demanded that they start using the more modern exceptions mechanism to handle errors.

    Separately, these same offshore developers found themselves in the middle of coding a function when they discovered that they needed to return multiple values. You can solve this by refactoring. If you are using "C", C++, etc, you can pass an out-arg by reference and populate it as a side affect (not desireable, but it works). You can even create a ReturnDataArgs object, populate and return it from the function.

    Not these guys. In a stroke of genius, they discovered a way to kill two birds with one stone:

      public class ExceptionException extends Exception {
    // implement all the Exception constructors as simple pass-thru's
    }

    public class SuccessException extends Exception {
    private int retVal1;
    private double retVal2;
    private String retVal3;
    public SuccessException(int a, double b, String c) {
    retVal1 = a;
    retVal2 = b;
    retVal3 = c;
    }
    public int getRetVal1() { return retVal1; }
    public double getRetVal2() { return retVal2; }
    public String getRetVal3() { return retVal3; }
    }

    // Implemented as:

    public Exception someFunc() {
    // Do stuff

    if (anErrorOccurred) {
    return new ExceptionException("Bad thing happened");
    }

    // Success
    return new SuccessException(1, 2.3, "Some String");
    }

    // And it's used like this:

    Exception rc = someFunc();
    if (rc instanceof ExceptionException) {
    log.error(rc.getMessage());
    return -1;
    }
    int i = ((SuccessException) rc).getRetVal1();
    double d = ((SuccessException) rc).getRetVal2();
    String s = ((SuccessException) rc).getRetVal3();



  • ShotGun - and I do not mean that I am calling out for a chance to ride in the front seat...



  • @snoofle said:

    In a stroke of genius

    I suppose if no one is willing to touch the code base that you support in an organization that it protects you from being replaced so it kinda is.  Of course competence would probably be a better method than what they are using.



  • So ... what you're saying ... is that they don't want to use exceptions when running into exceptional circumstances, but (ab)use exceptions as return values?

    Maybe I *don't* want to work with you after all.

    Maybe they should wrap the Reflection API with their own types while they're at it ... no, wait, sorry ... we do that here.

     



  • At least it's returning SuccessExceptions as the return value, rather than throwing them and hoping the caller has a catch clause set up to catch the "return" value.



  •  WHAT.

    Indian offshore  easy joke apart, I have seen a significant number of the more technical-minded school having a tendancy to use just about any language structure for something widly different, like memcache for keeping translation in memory (no chance the server crash ever, no ?), associative array as crude FIFO pile, or PHP as a programmaton language. This one take the cake though ; did they really have no clue what exception are supposed to be ?



  • @zelmak said:

    Maybe they should wrap the Reflection API with their own types while they're at it ... no, wait, sorry ... we do that here.

    "wrap the Reflection API with their own types" is about the same as "write classes that use Reflection to accomplish some legitimate purpose" in my experience. It only becomes a WTF when you've exposed most of the same operations as the original API, without adding any value such as conciseness, expressive power, or speed. Bonus WTF points for being incorrect or removing the ability to perform common tasks.


  • FoxDev

    @snoofle said:

      public class ExceptionException extends Exception {
    // implement all the Exception constructors as simple pass-thru's
    }

    public class SuccessException extends Exception {
    private int retVal1;
    private double retVal2;
    private String retVal3;
    public SuccessException(int a, double b, String c) {
    retVal1 = a;
    retVal2 = b;
    retVal3 = c;
    }
    public int getRetVal1() { return retVal1; }
    public double getRetVal2() { return retVal2; }
    public String getRetVal3() { return retVal3; }
    }

    Needs one more:

    public class HeisenbergException extends Exception {
    public bool Measure();

    //Normal exception stuff
    }


  • @TheLazyHase said:

    associative array as crude FIFO pile
     

    What the fuck? AssArrays don't even have sequence.



  • @TheLazyHase said:

    did they really have no clue
    These particular individuals barely have a clue how to breathe. If it's possible to do something in a stupid way, they'll find it.



  • @mott555 said:

    At least it's returning SuccessExceptions as the return value, rather than throwing them and hoping the caller has a catch clause set up to catch the "return" value.

    Well, there's nothing in Java that forces you to actually get the return value of a function - you can just ignore it, which in this case, is just as bad.

       // They did it like this:
    public Exception someFunc() { ... }

    someFunc(); // ignore success/failure

    Exception ignore = someFunc(); // almost as bad

    // if they had done it like this:

    public void someFunc() throws Exception { ... }

    try {
    someFunc();
    } catch (Exception e) {
    ; // just as wtf
    }

    try {
    someFunc()
    } catch (ExceptionException ee) {
    ; // just as WTF
    } catch (SuccessException se) {
    ; // just as WTF
    }



  • @snoofle said:

    like doing error checking in the old "C" way

    more modern exceptions mechanism to handle errors

    I'm not here to teach any one how to program but that a function/method exits with an error is not the same as throwing an exception, or to be more precise: error != exception

    Using exceptions to indicate that an expected error happened is not a good practice since they usually are expensive performance wise compared with returning 'null' for example. Also, using exceptions unwisely can lead to a tight coupled code, i.e. a view handling SqlException's is tight coupled to the persistence layer and you're doing something terribly wrong.

    http://www.amazon.com/Effective-Java-2nd-Joshua-Bloch/dp/0321356683



  • @jamesn said:

    @zelmak said:

    Maybe they should wrap the Reflection API with their own types while they're at it ... no, wait, sorry ... we do that here.

    "wrap the Reflection API with their own types" is about the same as "write classes that use Reflection to accomplish some legitimate purpose" in my experience. It only becomes a WTF when you've exposed most of the same operations as the original API, without adding any value such as conciseness, expressive power, or speed. Bonus WTF points for being incorrect or removing the ability to perform common tasks.

    As I don't have access to the original coders but do have access to the code, the way they did it appears to have simply removed all the various exceptions which can be thrown during a reflection operation (ClassNotFoundException, IllegalAccessException, InstantiationException, IllegalArgumentException, InvocationTargetException...) into a single home-made ReflectionException.

    My problem is that they used reflection so much that it was such a problem to use reflection that they needed this/these wrappers.



  • @dhromed said:

    @TheLazyHase said:

    associative array as crude FIFO pile
     

    What the fuck? AssArrays don't even have sequence.

    s/he DID say 'crude' ... :)



  • <FakeNagesh>

    THANK YOU SO MUCH FOR THIS CODES! WE WILL BE USING THIS TECHNIQUE IN FUTURE.

    </FakeNagesh>

     @snoofle said:

    While spelunking around in a section of our system I've not previously encountered, I discovered that our offshore developers like doing error checking in the old "C" way:

       int rc = someFunc();
    if (rc != 0) {
    return rc; // error
    }

    ...and then checking for it everywhere. We begged, pleaded, insisted, and finally demanded that they start using the more modern exceptions mechanism to handle errors.

    Separately, these same offshore developers found themselves in the middle of coding a function when they discovered that they needed to return multiple values. You can solve this by refactoring. If you are using "C", C++, etc, you can pass an out-arg by reference and populate it as a side affect (not desireable, but it works). You can even create a ReturnDataArgs object, populate and return it from the function.

    Not these guys. In a stroke of genius, they discovered a way to kill two birds with one stone:

      public class ExceptionException extends Exception {
    // implement all the Exception constructors as simple pass-thru's
    }

    public class SuccessException extends Exception {
    private int retVal1;
    private double retVal2;
    private String retVal3;
    public SuccessException(int a, double b, String c) {
    retVal1 = a;
    retVal2 = b;
    retVal3 = c;
    }
    public int getRetVal1() { return retVal1; }
    public double getRetVal2() { return retVal2; }
    public String getRetVal3() { return retVal3; }
    }

    // Implemented as:

    public Exception someFunc() {
    // Do stuff

    if (anErrorOccurred) {
    return new ExceptionException("Bad thing happened");
    }

    // Success
    return new SuccessException(1, 2.3, "Some String");
    }

    // And it's used like this:

    Exception rc = someFunc();
    if (rc instanceof ExceptionException) {
    log.error(rc.getMessage());
    return -1;
    }
    int i = ((SuccessException) rc).getRetVal1();
    double d = ((SuccessException) rc).getRetVal2();
    String s = ((SuccessException) rc).getRetVal3();



  • Luckily Java added the ability to chain Exceptions, so now you can make a PersistenceException class that stores a reference to the real SQLException while at the same time getting correct stacktraces.



  • Wait, you can't return a function-specific struct? Wow, C(++) is worse than I thought...



  • @Ben L. said:

    Wait, you can't return a function-specific struct? Wow, C(++) is worse than I thought...
     

    You can definitely return structs in C... I don't know what "function-specific struct" means though.



  • @too_many_usernames said:

    @Ben L. said:

    Wait, you can't return a function-specific struct? Wow, C(++) is worse than I thought...
     

    You can definitely return structs in C... I don't know what "function-specific struct" means though.

    Let's say my function returns a file pointer and the size of the file or an error. I should be able to return a struct { FILE* file; size_t length; ErrorCode error; }. Or at least multiple return values per function.



  • @Ben L said:

    Let's say my function returns a file pointer and the size of the file or an error. I should be able to return a struct { FILE* file; size_t length; ErrorCode error; }. Or at least multiple return values per function.

    You can return either a named type or (if the compiler is modern enough) an unnamed/anonymous type.



  • @TheCPUWizard said:

    Then why the anti-pattern of returning through a parameter? Unless the syntax for extracting from that anonymous struct is even more fucked up than most of C... Which would be another WTF for another day.



  • @dhromed said:

    @TheLazyHase said:

    associative array as crude FIFO pile
     

    What the fuck? AssArrays don't even have sequence.

     

    I've seen a Perl hash comprise of numbered keys, along with a routine to sort the keys numerically so that the values could be written out in a specific order.

    I've no idea why either. It wasn't the only WTF I encountered in that codebase.

     



  • @Ben L. said:

    @TheCPUWizard said:
    Then why the anti-pattern of returning through a parameter? Unless the syntax for extracting from that anonymous struct is even more fucked up than most of C... Which would be another WTF for another day.

    Out parameters are generally considered an anti-pattern because they require declararion of the variable to hold the result BEFORE the call to the method in question. Also multiple out parameters can get really ugly. It is generally considered "Best practices" to return results via the return value.

    note: remember, I used generally (twice) in the above....



  • @TheCPUWizard said:

    Out parameters are generally considered an anti-pattern because they require declararion of the variable to hold the result BEFORE the call to the method in question.
     

    Is that just a C thing?

    I would have thought declaring a variable of a specific data type was the expected responsibility of the caller, as defined by the method signature. Or am I misunderstanding the concept here?

    @TheCPUWizard said:

    Also multiple out parameters can get really ugly. It is generally considered "Best practices" to return results via the return value.
     

    Surely that's language-specific, though...?

    Just thinking of shell-scipting languages, where a status value and return value separate out whether or not the operation worked, and what results if successful.



  • In my opinion, it's less language specific than coding "style" (sorry, don't know the english term)

    If you are imperative-oriented , then no problem here ; in concept your function are just goto, so do whatever you want. You should not use exception (or object) however, for consistency and making people only pull out three quarter of their hair instead of everything. It's a bit old and inconvenient for big project, but it's not by itself a WTF ; it's more like using a cycle instead of a car.

    If you are function-oriented, passing return by anything else than return value is abhorrent. Your function is supposed to work in a vacuum and depend only on its arguments. In business, it's extremely rare ; it work very well when you do mathematics, but not too well for a lot of other use.

    If you are object oriented, then it's not a function but a method call, and you can change the state of the object instead of returning the value. It's more or less the "standard".

    Of course, I understand well that on most case people make a mix of object-oriented and imperative programming. That does not prevent you to be better off trying to stick on one category as much as possible, even if you prefer be a bit flexible on some cases to avoid pulling your hair. But of course of lot people have no clue that what imperative and object mean (I am sure someone will say I am one of them :p), and so they do that kind of horror.

    Another pretty one I have seen were a PHP class that had only static method, that were used as namespaced function. That were calling other method of other entirely static classes, with a 5-level deep marsh.



  • @Cassidy said:

    @TheCPUWizard said:

    Out parameters are generally considered an anti-pattern because they require declararion of the variable to hold the result BEFORE the call to the method in question.
     

    Is that just a C thing?

    I would have thought declaring a variable of a specific data type was the expected responsibility of the caller, as defined by the method signature. Or am I misunderstanding the concept here?

    The difference here - for C++ - is that an object needs to be constructed by the calling function. For example:

    // First example with value returned through parameter:
    SomeType type; // <-- SomeType constructor called
    someFunc(type, ...); // <-- type is used as output parameter here
    

    // Second example with return value:
    SomeType type = someOtherFunc();

    In the first example, the SomeType object needs to be constructible without any parameters, which is true for most types - but not for all! In the second case, the called function is responsible for creating the object, and it can pass any parameter to the constructor.



  • Cassidy,

    Your example requires (but does not show) that the parameter must be declared as pass by reference (&) otherwise somefunc will only be modifying a local copy...

    But that is still really an out parameter. See the following:

     

    void

    StrongConstraints( in clX paInParameter, //info that will be read but never written to
                                  out clX& paOutParameter, //info that will be written to
                                  in out clX& paInOutParameter) //info that will be read and written to

    in : parameter will be read (warning when not read in every branch*)
    (in): parameter can be read (warning when nowhere read)
    out: parameter will be written to (warning when not written to in every branch*)
    (out): parameter can be written to (warning when nowhere written to)
    in without out: parameter cannot be written to
    out without in: parameter cannot be read before it is written to.
    *in every branch: in all possible normal flows through the function (both branches in an if statement, all branches in a case statement,... ), but not when an exception is thrown. This is the same constraint as for a return statement, to have no undefined behavior.

    An input parameter makes all parts of the type of the parameter const, independent if they are explicitly declared const.
    An output or input/output parameter must have a type so that changing the parameter has effect outside the function.

    In an overruled virtual function, the constraints can be changed: in can become (in), (out) can become out, but never the inverse.



  •  That ExceptionException is a particularly nice touch, though, you'll have to admit.



  • @Ben L. said:

    I should be able to return a struct { FILE* file; size_t length; ErrorCode error; }.
     

     Please, just refrain from doing that. Give a name to your struct if it is part of the API.

    Your way makes the calling code unecessarly complex.



  • @TheCPUWizard said:

    Cassidy,

    Your example requires (but does not show) that the parameter must be declared as pass by reference (&) otherwise somefunc will only be modifying a local copy...

    But that is still really an out parameter. See the following:

     

    void

    StrongConstraints( in clX paInParameter, //info that will be read but never written to
                                  out clX& paOutParameter, //info that will be written to
                                  in out clX& paInOutParameter) //info that will be read and written to

    in : parameter will be read (warning when not read in every branch*)

    (in): parameter can be read (warning when nowhere read)

    out: parameter will be written to (warning when not written to in every branch*)

    (out): parameter can be written to (warning when nowhere written to)

    in without out: parameter cannot be written to

    out without in: parameter cannot be read before it is written to.

    *in every branch: in all possible normal flows through the function (both branches in an if statement, all branches in a case statement,... ), but not when an exception is thrown. This is the same constraint as for a return statement, to have no undefined behavior.

    An input parameter makes all parts of the type of the parameter const, independent if they are explicitly declared const.

    An output or input/output parameter must have a type so that changing the parameter has effect outside the function.

    In an overruled virtual function, the constraints can be changed: in can become (in), (out) can become out, but never the inverse.

    Is that some kind of weird, jumped-the-shark C++ 11 syntax or are you just adding your own stuff to the language?


Log in to reply