So I have this IOErrorCode record which encapsulates errors from the underlying API. The point is to be able to pass this error code to a user-callback, and so raising immediately is not desirable.

-
So I have this IOErrorCode record which encapsulates errors from the underlying API. The point is to be able to pass this error code to a user-callback, and so raising immediately is not desirable.

Modelling it after Boost's error_code it has an implicit conversion to boolean, where "no error" is mapped to False, otherwise True.

While it looks neat, I find it can be a bit obscure, and so want to remove it.

So, choices...

1. if (Success(ErrorCode)) then ...

2. if (not Failed(ErrorCode)) then ...

3. if (not ErrorCode.Failed) then ...

Thoughts? Other ideas?

Comments

  1. Jeroen Wiert PluimersJune 8, 2015 at 8:51 AM

    No poll? I prefer 3.

    ReplyDelete
  2. Paul TOTHJune 8, 2015 at 9:01 AM

    4. Error := ...

    where Error is a property with a setter that check the value for error condition :)

    ReplyDelete
  3. Asbjørn HeidJune 8, 2015 at 9:11 AM

    Jeroen Wiert Pluimers I wrote it on my phone, forgot about it (and no clear option anyway).

    Paul TOTH Sorry, I should have clarified, this type is for passing error values back to the user in places where always raising an exception is not desireable or possible.

    ReplyDelete
  4. David HeffernanJune 8, 2015 at 9:19 AM

    Success and Failed don't match. Succeeded and Failed would match. But those names are already used for COM error handling. Using a method provides encapsulation and avoids name space clashes. Always a good idea in a language that lacks name spaces.

    ReplyDelete
  5. Asbjørn HeidJune 8, 2015 at 9:24 AM

    David Heffernan Success/Failed was to explore variations on a theme. I recalled wrong on the tram then, thought COM used Success() and not Succeeded() :(

    New suggestion:

      if (not ErrorCode.IsFailure) then ...

    ReplyDelete
  6. David HeffernanJune 8, 2015 at 9:57 AM

    ErrorCode is a pretty bad name for something that can be a success. Status would be better.

    ReplyDelete
  7. Asbjørn HeidJune 8, 2015 at 10:05 AM

    David Heffernan Well, that'd be ERROR_SUCCESS ;)

    Status feels very unnatural for me, but I'll chew on it.

    ReplyDelete
  8. Thomas Mueller (dummzeuch)June 8, 2015 at 10:28 AM

    ResultCode then?
    And I'm with Jeroen Wiert Pluimers​ and David Heffernan​ : a method is better than a function, especially since it already is a record. I prefer . HasFailed

    ReplyDelete
  9. Asbjørn HeidJune 8, 2015 at 10:42 AM

    Thomas Mueller Well, the issue I have with Status or ResultCode is that to me they suggest it can be something else than an error. But that's not how it works. It's either an error or the absence of an error (aka success).

    ReplyDelete
  10. Heinz ToskanoJune 8, 2015 at 12:13 PM

    Limiting to boolean range seems odd to me, particularly when errors can have more than 2 possible values.

    If you want something to evaluate without raising an exception, then you need someway to be able to determine what happened. So you need a conditional method and a status code read only property.

    BTW, anything that is not true is a failure, don't mix results, bad recipe for troubles.

    ReplyDelete
  11. Asbjørn HeidJune 8, 2015 at 12:41 PM

    Heinz Toskano It's not limited, IOErrorCode.Value provides the (platform specific) error code, while IOErrorCode.Message provides the error message.

    The reason that "success" evaluates to false is natural in C/C++ and POSIX, as an error code of zero means success and on Windows ERROR_SUCCESS is zero (though not all calls use this). And in C/C++, zero is interpreted as false...

    So that's what the implicit conversion operator tries to capture.

    However as you say, it can be a bit confusing, hence I'd like a more explicit way.

    ReplyDelete
  12. Asbjørn HeidJune 8, 2015 at 12:49 PM

    FWIW I'm planning on providing some non-platform-specific "constants", for common errors such as file not found etc.

    ReplyDelete
  13. Lars FosdalJune 8, 2015 at 1:00 PM

    Here is my variant. It's no longer quite "pure", as it has evolved to fill varying needs.

      TPSDResult = (
         rOK,                 // Operation was ok
         rError,              // Operation was a failure
         rNothingPerformed,   // Operation did nothing
         rRemoved,            // Operation was
         rPartlyPerformed     // Operation was partly performed
         );

      TPSDResultSet = set of TPSDResult;

      /// Action on data during operation
      TPSDReason = (
         r_Unknown, // Normally not used
         r_None,
         r_Added,
         r_Updated,
         r_Removed,
         r_Ignored,
         r_Denied,
         r_Conflicted
         );

      TPSDResultEx = record
      private
        FStatus: TPSDResult;
        FReason: TPSDReason;
        FDefaultStatus: TPSDResult;
        FSuccessSet: TPSDResultSet;
        FDefaultSuccessSet: TPSDResultSet;
        FSuccessCount: Integer;
        FErrorCount: Integer;
        FLastXMLLogId: Integer;
        FLastSuccessMessage: String;
        FLastErrorMessage: String;
        FResultCount: Integer;
      public
        constructor Create(const aDefaultStatus: TPSDResult);
        procedure Reset;
        procedure SetSuccessSet(const Value: TPSDResultSet);
        procedure SetDefaultSuccessSet(const Value: TPSDResultSet);
        procedure SetDefaultStatus(const Value: TPSDResult);
        procedure SetFail(const aLastErrorMsg: String = ''; aReason:TPSDReason = r_Unknown; aLastXMLLogId: Integer = -1);
        procedure SetSuccess(const aLastSuccessMsg:String = ''; aLastXMLLogId: Integer = -1);
        procedure IncSuccessCount;
        procedure IncErrorCount;
        function PartlyPerformed: Boolean;
        function Fail: Boolean;
        function Success: Boolean;
        property Status:TPSDResult read FStatus write FStatus;
        property DefaultStatus:TPSDResult read FDefaultStatus write SetDefaultStatus;
        property SuccessSet: TPSDResultSet read FSuccessSet write SetSuccessSet;
        property DefaultSuccessSet: TPSDResultSet read FDefaultSuccessSet write SetDefaultSuccessSet;
        property Reason: TPSDReason read FReason;
        property LastXMLLogId: Integer read FLastXMLLogId;
        property LastErrorMessage:String read FLastErrorMessage;
        property SuccessCount: Integer read FSuccessCount;
        property ErrorCount: Integer read FErrorCount;

        ///Can be used to pass a resulting integer value. Ex: Number of affected rows
        property ResultCount: Integer read FResultCount write FResultCount;
      end;

    ReplyDelete
  14. Asbjørn HeidJune 8, 2015 at 1:07 PM

    Lars Fosdal That's a mighty cannon for my moskito :D

    ReplyDelete
  15. Dorin DuminicaJune 9, 2015 at 12:23 AM

    like David Heffernan said above, if [IOErrorCode] can return Success or Fail, then, I'd reconsider naming it to [IOResult] or [IOExecResult] that is a struct with an enum field [Status] that can be [kECSuccess, kECFail, kECUnknown], being a struct, you can expand and add more details in the future without breaking current apps

    just my 2.15 cents

    ReplyDelete
  16. Asbjørn HeidJune 9, 2015 at 1:18 PM

    Thanks for the input guys, good stuff.

    ReplyDelete

Post a Comment