SAP / styleguides

This repository provides SAP style guides for coding and coding-related topics.
Other
1.66k stars 443 forks source link

Should we use plural for returning parameter? #190

Open lucasborin opened 3 years ago

lucasborin commented 3 years ago

The thread starts here: https://github.com/SAP/code-pal-for-abap/issues/335

The reporter said:

We have an internal inconsistency anyways between the two requirements

  1. the method should return "result"
  2. tables should be plural, i.e. when a method returns a table, it should return "results" rather than "result"

IMO, the Method's name should say if it returns one entry or the whole table, not the parameter itself. For instance: get_child( ) or get_children( ).

Could you please share your inputs here?

pokrakam commented 3 years ago

Agree, to me result is a collective noun in this context, the result of the method. It affirms that it is a functional method with one and only one returning parameter. It's a bit like the distinction between hair and hairs ("she has red hair"), or people and peoples. results to me indicates a heterogeneous collection, like having a series of medical tests and getting the results back. The exporting parameters and tables of a function module are its results. Edit: If it bothers someone that much one could make a convention to use result right at the end, JavaScript style. result = lt_processed_records is perfectly clear.

pokrakam commented 3 years ago

Another 'style' thing may be worth mentioning as it's in a similar line: I personally don't like using result too many times in a method. So when it comes to:

METHOD get_widget_with_details.
  result = NEW zcl_widget( ). 
  result->attr2 = get_some_value( ). 
  SELECT... 
  LOOP AT ...
    result->add_data( ... ).
  ENDLOOP. 
ENDMETHOD.

It reads awkward to me. If I have to work with result more than once or twice I usually use an intermediate.

METHOD get_widget_with_details.
  data(widget) = NEW zcl_widget( ). 
  widget->attr2 = get_some_value( ). 
  SELECT... 
  LOOP AT ...
    widget->add_data( ... ).
  ENDLOOP. 
  result = widget.
ENDMETHOD.
HrFlorianHoffmann commented 3 years ago

Share @pokrakam's opinion above. My team uses result as a non-countable noun. It represents fields, structures, and tables alike. All of those are simply "the result". A table of people, documents, identifiers etc. is still only one result.

RolfMantel commented 3 years ago

I see a significant difference between get_addresses() returning a table of addresses and find_addresses() returning a structure with addresses and search_status.

If I use the java-script style of introducing a local variable, I reduce half of the aim of the rule, namely that the meaningful name for the return parameter is mostly the same as the method name, and no, in the example above, widget is not an acceptable variable name inside a method get_object_with_details, I could name the object "peter" with the same amount of information (it would be an acceptable variable name inside a method get_widget).

pokrakam commented 3 years ago

@RolfMantel I think you're missing the point in both my examples. First off, they are unrelated points. JavaScript does not specifically mandate the use of a local variable, it's just the way the syntax works. You exit a function with return <value>;. So I made a suggestion that in a similar vein we could use a descriptive name and assign it to result at the end if someone feels there is a contradiction.

My second post was really just about a personal preference. The main point was not on having the 'right' variable name, but on not using the generic 'result' if further work needs to be done before returning. In my example called it widget because it was a class zcl_widget. You are right to point out that the method name doesn't match, and that was a poor example on my part. I have edited it to rename get_object_with_details to get_widget_with_details.

RolfMantel commented 3 years ago

Sure, the point is on using the generic result. In what way does the variable name result make the code more readable than a carefully chosen variable name?

My personal interpretation of the guideline to use the generic result is that the purpose of this rule is to simplify matters for the reader: here, we are doing some preparatory work, there we are technically building up the result.

In your example above, using result from the first line of the method, I see at first glance that I am constructing the result step by step. In the second variant, I need to look very carefully whether the widget that I construct step by step is going to be the result or whether the widget that I constructed in the beginning is going to end up only being a sub-object of the final result.

HrFlorianHoffmann commented 3 years ago

The original question was whether we should prefer singular "result" or plural "results" when returning tables. So far, looks like most agree that it should remain "result" in this case.

The question whether we should reference returning and exporting parameters from the beginning on or prefer intermediate local variables and assign them only at the very end is a different question. Should we clarify that by moving that discussion out into a separate issue?

RolfMantel commented 3 years ago

I think those two are connected. If we prefer intermediate local variables, there is little difference between result and results.

Only if we reference returning parameters from the beginning, we will see the difference between append get_address() to results.
result = get_address( ). append get_address( ) to result->addresses.

lucasborin commented 3 years ago

Please, consider this thread for non-tables as well. In the code-pal, the check applies for all kinds of returnings (object, table, string, etc).

HrFlorianHoffmann commented 3 years ago

As we have seen above, people tend to use "result" in an uncountable way. result then can refer to a single number or a string as well as a table of documents or an object that represents a list of addresses.

Meaning if the method's name is clear, we would see:

 METHOD get_persons.
   result = VALUE #( ( `Alice` ) ( `Bob` ) ... ).
 ENDMETHOD.

And if the method's name is hazy, we would find a parameter name that describes its content better:

METHOD filter.
   matching_persons = VALUE #( ( `Alice` ) ( `Bob` ) ... ).
ENDMETHOD.

What we would see less is:

 METHOD get_persons.
   results = VALUE #( ( `Alice` ) ( `Bob` ) ... ).
 ENDMETHOD.

Whereas the word itself is not unwelcome and I could imagine rare but valid cases for using it:

 METHOD calculate_fractions.
   results =
     VALUE #(
       FOR formula IN input
         ( formula-dividend / formula-divisor ) ).
 ENDMETHOD.

Whether we are writing

 METHOD get_persons.
   INSERT `Alice` INTO TABLE result.
   INSERT `Bob` INTO TABLE result.
   ...
 ENDMETHOD.

or

 METHOD get_persons.
   DATA persons TYPE string_table.
   INSERT `Alice` INTO TABLE persons.
   INSERT `Bob` INTO TABLE persons.
   ...
   result = persons.
 ENDMETHOD.

however still seems unrelated to me. Both cases still use "result" and I wouldn't choose the plural in either of them.

Ennowulff commented 1 year ago

A method returns a "result". No matter, how this result looks like, how many table lines, fields or attributes there are. If there were "results" then this would result in multiple (exporting) parameters. In this case the results must be named anyway. jm2c