2020 Aug 06 12:48 PM
Hey all!
There is an option in the Code Inspector that warns about unhandled exceptions like
TRY.
get_an_optional_value( ).
CATCH value_not_maintained.
ENDTRY.
How should one handle the CATCH part?
I often see comments like
"nothing to do
or
"expected error; value remains
Another option is to place a "useless" command:
my_structure-field = my_structure-field.
To fulfill the requirement to handle exceptions in any case I also often see:
lcl_exceptions_helper=>nothing_to_do_here( ).
If you leave an empty CATCH block - what is another option - then the next developer might be insecure if you forgot to handle the exception.
What is your opinion to handle an exception that is no "error" but just a technical way to say "there is no result, sorry"?
Hey all!
There is an option in the Code Inspector that warns about unhandled exceptions like
TRY.
get_an_optional_value( ).
CATCH value_not_maintained.
ENDTRY.
How should one handle the CATCH part?
I often see comments like
"nothing to do
or
"expected error; value remains
Another option is to place a "useless" command:
my_structure-field = my_structure-field.
To fulfill the requirement to handle exceptions in any case I also often see:
lcl_exceptions_helper=>nothing_to_do_here( ).
If you leave an empty CATCH block - what is another option - then the next developer might be insecure if you forgot to handle the exception.
What is your opinion to handle an exception that is no "error" but just a technical way to say "there is no result, sorry"?
2020 Aug 06 12:54 PM
Exception is always complex, most of people use them because they have to do it, but not for real reason.
I wonder why do you catch this exception ? do you want to do something specific ? no ? so why do you need to catch it ?
If you declare Value_Not_Maintained in the method definition, it is not enough ?
Edit: For me Exception should be used, when you need to infom the calling method, there is a problem, you have to manage it. Otherwise I never use Exception.
This rule, is linked to the rule to always use RETURN parameter. As there is only one RETURN paramter, EXCEPTION is the only way to inform, I have a problem (because empty value is a lot of time a possible value).
Example:
do I have to say the material is OK / NOT-OK, or do I have to say the material is not in the db
method is_material_ok.
try.
rv_result = cond #( when get_material_group( material_number ) eq 'Dummy
then abap_true
else abap_false ).
catch zcx_material_rules into data(lo_exceptio).
raise exception zcx_material_rules
textid = zcx_material_rules=>material_control_not_possible
previous = lo_exception.
endtry.
endmethod.
2020 Aug 06 2:51 PM
I agree with Frederic. The method shouldn't be throwing an exception for an optional value not being maintained. Better would be to change it so it doesn't throw an exception, and write something like:
IF value_is_maintained.
get_an_optional_value( ).
ENDIF.But there are times when you have to catch an exception but don't care if its triggered. In this case, KISS is the way to go and use a comment. Code that doesn't do anything is just confusing. However, I always catch the exception into an instance variable as if debugging is necessary it gives additional information.
CATCH cx... INTO DATA(error).
" Do nothing, because it can be not maintained
ENDTRY.
2020 Aug 06 3:05 PM
If absolutely nothing needs to be done pragma ##NO_HANDLER can be used to hide code check warnings. I prefer to accompany it with a "nothing to do" comment, to let the next developer know that the handler section is deliberately empty, not just out of laziness.
Writing unnecessary source code just to fill the space is a big no-no.
2020 Aug 06 4:50 PM
frdric.girod matthew.billingham Thanks for your answers!
I think my example could have been better. Maybe the following variant makes it clearer:
TRY.
my_already_filled_variable = get_an_optional_value( ).
CATCH value_not_maintained.
ENDTRY.
Now the exception makes sense because the return parameter will not be filled thus the variable will not be set.
If I want to solve this problem w/o exceptions then it would look something like this:
data(result) = get_an_optional_value( ).
IF result IS NOT INITIAL.
my_already_filled_variable = result.
ENDIF.I find this variant less elegant than the one using the exception class. Additionally: the caller must know about the "initial" value; "SPACE" might be a valid parameter value.
gabmarian thanks for the hint with the pragma! that's a good solution. Don't know why I didn't think of that...
2020 Aug 06 5:55 PM
I think there should always be the Exception that no value is found. Especially if you want to reuse the Code. Where in one Situation this is a critical error while in another Situation this is tolerable. In any case the caller is informed that there the result is undefined and not initial (null versus initial). If I remember correctly the return variable is always cleared when raising an Exception.
So to answer your question: I would use the pragma but in our System I have to use a special Method call that does nothing.
2020 Aug 06 9:02 PM
Really it depends on your philosophy of the use of exceptions. I try to only use them for error situations. If it isn't an error, don't throw an exception.
IF it's ok.
Do it.
Endif.Is clear.
Try.
Do it.
Catch error.
ignore.
Endtry.Is not.
2020 Aug 07 9:46 AM
matthew.billingham it depends on the point of view, what an error is. the function might not know. It depends on the process, the caller of the function.
Example: There is a document and containing some longtexts. maybe... In your application you call a method GET_TEXT to display it in your application. It doesn't matter if that text does not exist.
TRY.
optional_textlines = get_text( order = '1234' id = 'ZZ01' ).
CATCH not_found.
"doesn't matter: no text - no problem.
ENDTRY.In another context you want to make sure that this specific longtext exists. You call the same function but now you have to react to this exception:
TRY.
optional_textlines = get_text( order = '1234' id = 'ZZ01' ).
CATCH not_found.
MESSAGE 'Text is missing!!!' TYPE 'E'.
ENDTRY.
The function get_text itself cannot know the callers intention.
2020 Aug 07 10:07 AM
michael.biber2 If I remember correctly the return variable is always cleared when raising an Exception.
Maybe it works like that in function modules. I am not sure. with methods the behaviour is the following:
class based exception
old exception
CLASS not_found DEFINITION INHERITING FROM cx_static_check.
ENDCLASS.
CLASS main DEFINITION.
PUBLIC SECTION.
METHODS get_value_1
RETURNING
VALUE(output) TYPE i
RAISING
not_found.^
METHODS get_value_2
EXPORTING
output TYPE i
RAISING
not_found.
METHODS get_value_3
RETURNING
VALUE(output) TYPE i
EXCEPTIONS
not_found.
METHODS get_value_4
EXPORTING
output TYPE i
EXCEPTIONS
not_found.
METHODS start.
ENDCLASS.
CLASS main IMPLEMENTATION.
METHOD get_value_1.
output = 111.
RAISE EXCEPTION TYPE not_found.
ENDMETHOD.
METHOD get_value_2.
output = 222.
RAISE EXCEPTION TYPE not_found.
ENDMETHOD.
METHOD get_value_3.
output = 333.
RAISE not_found.
ENDMETHOD.
METHOD get_value_4.
output = 444.
RAISE not_found.
ENDMETHOD.
METHOD start.
DATA(myvalue) = 1.
TRY.
myvalue = get_value_1( ).
CATCH not_found.
ENDTRY.
cl_demo_output=>display_data( myvalue ).
myvalue = 2.
TRY.
get_value_2( IMPORTING output = myvalue ).
CATCH not_found.
ENDTRY.
cl_demo_output=>display_data( myvalue ).
* myvalue = 3.
* myvalue = get_value_3(
* exceptions
* not_found = 1 ).
*
* cl_demo_output=>display_data( myvalue ).
myvalue = 4.
get_value_4(
IMPORTING
output = myvalue
EXCEPTIONS
not_found = 1 ).
cl_demo_output=>display_data( myvalue ).
ENDMETHOD.
ENDCLASS.
START-OF-SELECTION.
NEW main( )->start( ).
2020 Aug 07 1:02 PM
enno.wulff
That's wierd. I tested more or less exactly that (but with a returning parameter set to abap_true). On my system the target variable is not updated (myvalue in your case). It stays at abap_false.
Has that behavior been changed at some point in time? I tested that on a Netweaver 7.50 with latest SPS.
2020 Aug 07 1:27 PM
michael.biber2 that's really weird. I just copied the test report and tried with "CHAR01" and the result is the same...
2020 Aug 07 1:39 PM
What is really strange, in Clean-Code point of view, the method must have one responsibility and only one.
If the method has to answer exception, it should be an information.
If the method calling this method does not care about this information. Maybe it means the calling method change the responsability of the called method.
(yep I have take a coffee after lunch break)
(I really like this discussion, thank for that 🙂 )
2020 Aug 07 1:41 PM
enno.wulff The thing is: The behavior on my system is exactly as I would expect it. If there was an exception, don't use the result! It is invalid. And because of that I would always create an exception if nothing was found because there is the distinction between valid empty and invalid empty.
2020 Aug 07 2:31 PM
You said "Additionally: the caller must know about the "initial" value; "SPACE" might be a valid parameter value."
There was a recent discussion about returning a fully-typed data reference (i.e. not "type ref to data", but "type ref to <complete type>") instead of returning the value, to use IF result IS BOUND instead of IF result IS INITIAL:
DATA(result) = get_an_optional_value( ). " <==== returns a reference
IF result IS BOUND. " <==== IS BOUND
my_already_filled_variable = result->*. " <==== dereferencing
ENDIF.
2020 Aug 09 10:14 PM
You can remove the code inspector warning by adding a pragma ##NO_HANDLER

| User | Count |
|---|---|
| 4 | |
| 2 | |
| 2 | |
| 1 | |
| 1 | |
| 1 | |
| 1 | |
| 1 | |
| 1 | |
| 1 |