[MOAB-dev] Discussion on return type of MOAB routines

[Wu, Danqing]

Below is the summary of the discussions on return type so far. I notice that how we handle special "non-success" condition (like TAG_NOT_FOUND) will affect our decision. Any decision has its pros and cons. Maybe we need more opinions from other people. 

As for existing ErrorCode return type, Vijay and I prefer to use it. Iulian also voted to keep it simple (enum/int)
1) This does not break any existing user code nor does it need to change existing method signatures
2) According to Jed, PETSc returns error code because it is simple and portable between languages
3) Benchmarking tests show that it has least overhead for function return
4) No need to store any context explicitly, including error messages that are already printed out in the stack trace

Tim has some other reasons to return a class object instead of a plain enum type
5) The overhead might be acceptable for MOAB
6) For non-success conditions, we might need extra information hat can only come from the lower-level functions.
Regarding 6), Vijay and I have similar opinion:
Is that extra information in the returned class object really necessary? An upper level caller, based on some context, usually knows how to handle a non-error condition. It can choose to treat is as an expected error, and set it. It can choose to handle it and then return success. It can also keep it unhanded, and return this condition code unchanged to its callers. Of course, the lowest routines, like tag_get_handle(), will never set it as an error (because it does not know, totally decided by higher level callers), and just returns this code as is. In this case, an ErrorCode still would suffice. Using an object to store any context explicitly is probably not necessary here.

BTW, according to Jed, PETSc never uses error handlers/return codes for non-error conditions. It defines what should happen in that case and have a flag return if the caller needs to know.

Regarding returning a class object, there are three different options:
1) Use ostringstream as a member. This has more overhead according to benchmarking tests. The size of the object is 360 bytes.
2) Use std::string as a member. This is slightly better than a). The size of the object is 16 bytes.
3) Make the class a POD type, like C style struct, without constructors. Then char* is used as a member. A global heap space should be allocated to store the string. The issue is how to make this thread safe.
For now, it is more likely that we will use either 2) or 3)

Best
Danqing
________________________________________
From: Tautges, Timothy J.
Sent: Wednesday, January 22, 2014 3:36 PM
To: Vijay S. Mahadevan
Cc: Jed Brown; Wu, Danqing; moab-dev at mcs.anl.gov
Subject: Re: [MOAB-dev] Discussion on return type of MOAB routines

On 01/22/2014 01:34 PM, Vijay S. Mahadevan wrote:
>> It's entirely dependent on the application, same API call, same return,
>> different handling, and in some cases the application is a tool inside the
>> library.
>
> If the calling function knows how to handle the error based on the
> context i.e., fail with an error message or return with an error code
> to be handled by upper level routines, an ErrorCode still would
> suffice. Using an object to store any context explicitly is not be
> necessary here IMO.
>

But it's information from lower-level functions that will go missing.  Maybe that's not enough information to really
worry about, esp. if we have a stack trace, but that's the reason I'm saying we need this string.

- tim

> If you just want the control to augment the printed error message (in
> fatal failure cases), use macros like SETERR that take a stream. Since
> the program is going to exit due to the failure, each calling routine
> can print its message and return appropriately without worrying about
> the actual cost of storage of strings etc. For successful exits,
> string remains null. The const char* version in my timing results will
> be appropriate here.
>
> Vijay
>
> On Wed, Jan 22, 2014 at 1:26 PM, Tim Tautges (ANL) <tautges at mcs.anl.gov> wrote:
>>
>>
>> On 01/22/2014 01:21 PM, Jed Brown wrote:
>>>
>>> "Tim Tautges (ANL)" <tautges at mcs.anl.gov> writes:
>>>>
>>>> This depends on how often it happens.  In all cases, IMO, a success
>>>> condition should result in nothing more than a
>>>> POD-sized object (and I should add, no virtual table).
>>>
>>>
>>> vtables are just pointers like any other in terms of performance.
>>>
>>
>> Sure but we still don't want that extra expense.
>>
>>
>>>> If it's a non-success condition that is "expected" (normal condition
>>>> that's handled by the calling code), IMO this shouldn't print anything
>>>> by default.  If it's a non-success that's also unexpected, printing is
>>>> fine (including stack trace).
>>>
>>>
>>> I don't believe in expected failures.  I find keeping expected and error
>>> conditions in-band leads to confusing control flow, but YMMV.
>>>
>>
>> It's entirely dependent on the application, same API call, same return,
>> different handling, and in some cases the application is a tool inside the
>> library.
>>
>> - tim
>>
>>
>> --
>> ================================================================
>> "You will keep in perfect peace him whose mind is
>>    steadfast, because he trusts in you."               Isaiah 26:3
>>
>>               Tim Tautges            Argonne National Laboratory
>>           (tautges at mcs.anl.gov)      (telecommuting from UW-Madison)
>>   phone (gvoice): (608) 354-1459      1500 Engineering Dr.
>>              fax: (608) 263-4499      Madison, WI 53706
>>

--
================================================================
"You will keep in perfect peace him whose mind is
   steadfast, because he trusts in you."               Isaiah 26:3

              Tim Tautges            Argonne National Laboratory
          (tautges at mcs.anl.gov)      (telecommuting from UW-Madison)
  phone (gvoice): (608) 354-1459      1500 Engineering Dr.
             fax: (608) 263-4499      Madison, WI 53706