Sanity check: Is my simple method with an interface to select table data okay or "overkill"?

[BaerbelWinkler]

Hi Folks,

as many of you know, I'm not really on speaking terms with ABAP OO yet, so struggle with even the simplest tasks as I'm never quite sure if what I'm doing is the proper and/or best way to do it (even after checing stuff in Clean Code or various books). So, here is what I'd like to do and what I already coded for it followed by a few "sanity check" questions.

We have a generic parameter table - let's call it ZPARAMETERS - which is used for lots of stuff like deciding if a routine should be executed if there's an entry for specific values or to provide a certain value for a field. It's not pretty but it's not going to go away anytime soon. There are various ways in which the table gets queried and I'd at least try to get that a bit more streamlined. Which is why I dabbled a bit with functional methods.

Created an interface:

interface zif_table_zparameters_dao
  public .

  METHODS:
    entry_exists
      IMPORTING
        i_zparameters TYPE zparameters
      RETURNING
        VALUE(entry_found) TYPE abap_bool,

    return_value2
      IMPORTING
        i_zparameters TYPE zparameters
      RETURNING
        VALUE(result) TYPE zparameters-value2.

endinterface.

Global class implementing the interface:

CLASS zcl_table_zparameters DEFINITION
  PUBLIC CREATE PUBLIC.

  PUBLIC SECTION.
    INTERFACES zif_table_zparameters_dao.
  PROTECTED SECTION.
  PRIVATE SECTION.
ENDCLASS.

CLASS zcl_table_zparameters IMPLEMENTATION.

  METHOD zif_table_zparameters_dao~entry_exists.
    CLEAR entry_found.

    SELECT SINGLE @abap_true
         FROM zparameters
         INTO @entry_found
        WHERE modul        EQ @i_zparameters-modul
          AND z_program    EQ @i_zparameters-z_program
          AND field        EQ @i_zparameters-field
          AND value1       EQ @i_zparameters-value1
          AND value2       EQ @i_zparameters-value2.

  ENDMETHOD.

  METHOD zif_table_zparameters_dao~return_value2.
    CLEAR result.

    SELECT SINGLE value2
         FROM zparameters
         INTO @result
        WHERE modul        EQ @i_zparameters-modul
          AND z_program    EQ @i_zparameters-z_program
          AND field        EQ @i_zparameters-field
          AND value1       EQ @i_zparameters-value1.

  ENDMETHOD.

ENDCLASS.

Example code of how this will be used in programs, exits and enhancements:

DATA: int_zparameters TYPE REF TO zif_table_zparameters_dao.
DATA(cls_zparameters) = NEW zcl_table_zparameters( ).

int_zparameters ?= cls_zparameters.

TYPES: l_zparameters TYPE zparameters.

"Simply check if an entry exists
IF int_zparameters->entry_exists( VALUE l_zparameters( modul     = 'SD'
                                                       z_program = 'ZPROG'
                                                       field     = 'FIELD'
                                                       value1    = 'VALUE1'
                                                       value2    = 'VALUE2'
                                                   ) ).
  "Do whatever should be done if an entry is found
ENDIF.

"Return VALUE2 if the other fields are provided 
DATA(value2) = int_zparameters->return_value2( VALUE l_zparameters( 
                                                       modul     = 'SD'
                                                       z_program = 'ZPROG'
                                                       field     = 'FIELD'
                                                       value1    = 'VALUE1'
                                                   ) ).
IF value2 IS NOT INITIAL.
  "Use VALUE2 as needed   
ENDIF.

So, here are my "sanity check" questions:

1. Is the interface even needed for what I'm doing here - esp. as it's highly unlikely that there'll be unit tests for the logic? The main reason I created it, is that this seems to be the recommended way for public classes/methods.
2. Does the naming for the different entities make sense?
3. How to let the calling routine know that no entry might have been found in RETURN_VALUE2 in a functional method only returning exactly one value, in this case the content of VALUE2? At the moment I get around that by querying if VALUE2 is filled but I'm not sure if that't the best/proper way to do it.
4. Anything else you noticed?

Thanks much and Cheers

Bärbel

[Sandra_Rossi]

I wouldn't abstract if there's no reason to do it. Recommendations exist only for good reasons and I don't think it applies to your case.

If you have a good reason to use them, I would rename the methods to (I guess everyone know what DB means):

• db_entry_exists
• db_select

For returning the information found/not found, it's up to you. Using the initial value doesn't bother me, accompanied with a comment. Using an exception could be valid too.

[MateuszAdamus]

Hello 8b889d0e8e6f4ed39f6c58e35664518f

Here's my two cents on the topic. Clearly, other developers may have a different view.

1. Interface - I, for one, would not create it. Sure, there is an Interface-based Programming, but I really don't see a point in creating something that gives no real value. If you're only having one concrete implementation and you're using it only in your own systems, then, in my opinion, interface gives nothing.
If there is ever a need to have another implementation of the logic that reads values, then sure, interface might be a way to go, but you can refactor the code then, based on your needs.

2. Naming - is the ZPARAMETERS table the one and only table of that sort? If so, then I'd probably name interface/class as something more generic, maybe ZIF/ZCL_CONFIG or ZIF/ZCL_PARAMETERS.
Then the methods could be ENTRY_EXISTS (or simply EXISTS) and RETURN_VALUE (or VALUE). In the latter case I presume you always want to return the VALUE2 field's value, as the VALUE1 is a value used for comparison. Correct?
In any case, I'd try to keep the naming clear and short (but still meaningful). The reason for that is that this class and methods will be used a lot. It's good to have it easy to write and remember, so that one does not have to look for these every time.

3. How to let know - in my solution of such sort I made an EXPORTING parameter which returned TRUE/FALSE based on record existence. This way developer can (but doesn't have to) request for this information.

return_value2
  IMPORTING
    i_zparameters TYPE zparameters
  EXPORTING
    ev_found TYPE flag
  RETURNING
    VALUE(result) TYPE zparameters-value2.

METHOD zif_table_zparameters_dao~return_value2.
  CLEAR result.

  SELECT SINGLE value2
       FROM zparameters
       INTO @result
      WHERE modul        EQ @i_zparameters-modul
        AND z_program    EQ @i_zparameters-z_program
        AND field        EQ @i_zparameters-field
        AND value1       EQ @i_zparameters-value1.
  
  ev_found = xsdbool( sy-subrc = 0 ).
ENDMETHOD.

Example usage (sort of a pseudo code):

DATA:
  lv_found TYPE flag,
  lv_value TYPE string.

lv_value = lcl_config=>value( ).
lv_value = lcl_config=>value( IMPORTING ev_found = lv_found ).

You can see that in the first line developer does not care if the value is found or not. Maybe the returned value is a boolean, where an empty, default value, is fine with the logic.
But in the second case whether the value was found or not clearly has a meaning.

4. Anything else
4.a. Buffering? Do you have the table buffered or should the class buffer the data in case of multiple reads? Maybe you should buffer the whole instances of the class (using a factory method to get these)?
4.b. I've noticed you're giving all information about the parameter each time you want to read the parameter. Maybe you could create an instance of the logic with basic data (like modul, z_program) and then, when checking/reading data, provide only the FIELD (and VALUE1, if needed) values? This way the calls will be simpler. Also, less possibilities to make a typo when entering the modul/z_program names, as these would be entered only once.

DATA(lo_zparameters) = NEW zcl_table_zparameters( modul = 'SD' z_program = 'ZPROG' ).

"Simply check if an entry exists
IF lo_zparameters->entry_exists( field = 'FIELD' value1 = 'VALUE1' value2 = 'VALUE2' ).
ENDIF.

Kind regards,
Mateusz

[BaerbelWinkler]

mateuszadamus

Thanks, Mateusz!

Adding the EXPORTING parameters sounds like a straightforward solution for my 2nd method. But, wouldn't this be "against" this clean code principle stating to use either RETURNING or EXPORTING or CHANGING? Or would that be an elligble exeption to the rule in order to not let the perfect be the enemy of the good?

As for buffering: this isn't really a concern right now as the parameters table gets read literally "on end" throughout e.g. the large code of an enhancement with different WHERE-clauses including the values for MODUL and Z_PROGRAM, so there isn't much point in even trying to determine what this "basic data" might even be!

Cheers

Bärbel

[MateuszAdamus]

Hi 8b889d0e8e6f4ed39f6c58e35664518f

Definitely make the methods as simple as possible. But ease of use and performance are a factor, as well.

I think that it would be an overkill to have a separate method only to check if the value exists. Raising an exception is not perfect, too, because you always have to consider the exception being raised, i.e. you always have to call the method within the TRY CATCH clause, just to avoid possible exception and short-dump. And sometimes, the default value example, you won't care if the record is in the DB or not.

Hence the EXPORTING parameter, you can use it - but you don't have to, and it gets its value based on the SY-SUBRC of already made SELECT statement, so no unwanted overhead.

There is also another option - an attribute of a class, which would store the information, whether the record exists in DB or not. But this might get a bit messy, if the class gets more methods (logic) in the future.

Kind regards,
Mateusz

[MateuszAdamus]

PS I guess, you could also do it like they propose in the Clean ABAP in the "one parameter" section, I mean the part with returning a structure. Your method could then look like this.

public.
  TYPES:
    ygs_result,
      value TYPE zparameters-value2,
      exists TYPE flag,
    ygs_result.

  METHODS:
    return_value2
      IMPORTING
        i_zparameters TYPE zparameters
      RETURNING
        VALUE(result) TYPE ygs_result.

METHOD return_value2.
  SELECT SINGLE value2
       FROM zparameters
       INTO @result-value
      WHERE modul        EQ @i_zparameters-modul
        AND z_program    EQ @i_zparameters-z_program
        AND field        EQ @i_zparameters-field
        AND value1       EQ @i_zparameters-value1.
  
  result-exists = xsdbool( sy-subrc = 0 ).
ENDMETHOD.

" method call
DATA(ls_result) = lo_object->return_value2( ).
IF ls_result-exists = abap_true.
  DATA(lv_local_value) = ls_result-value.
ELSE.
  lv_local_value = 'SOME DEFAULT VALUE'.
ENDIF.

Kind regards,
Mateusz

[matt]

In rebuttal... 😉

1. Coding to interfaces is always a good idea, simply to always have the ability to mock it. (Ha! Ha!) If you design it in from the start, you won't need to refactor should the need arise. Which means your productive code will have been in the real world for a lot longer. Refactoring, though low risk, is not no risk. Safer to do it "properly" the first time.

2. Exceptions are most certainly not a bad thing because you have to code for them. It is no different, conceptually, from checking sy-subrc NE 0. Having said that, I prefer to use exceptions only for error conditions - not, for example, a read failing [ 1 ]... Exceptions are good because you can a) give them a payload and b) ignore them until the level that's appropriate for dealing with it is reached.

You seem to be arguing that in simple programs, you don't need these things - which I agree is within the KISS paradigm. I think that if you use these techniques even for simple tasks, you'll in the long run save a lot of time and effort.

TL;DR It "only being a simple thing" is a bad reason to, e.g. not use exceptions or interfaces. Use them as a matter of course and they're there when you need them. Creating programs is a lot cheaper than maintaining them.

Btw - naming of local classes etc., I use

• LCL - local class
• LIF - local interface
• LTD - test double
• LTC - test class

[MateuszAdamus]

Hi matthew.billingham ,

1. I think that sometimes interfaces are required, sometimes they are good to have and sometimes they are not needed. There's no one good answer. You're not able to predict all possible requirements for a solution and having interfaces won't save you the need for refactoring, if a new requirement is raised. Example from question's code:

DATA: int_zparameters TYPE REF TO zif_table_zparameters_dao.
DATA(cls_zparameters) = NEW zcl_table_zparameters( ).

int_zparameters ?= cls_zparameters.

"Simply check if an entry exists
IF int_zparameters->entry_exists( ....

What good does the interface bring here? If a need for additional implementation of the ZIF_TABLE_ZPARAMETERS_DAO interface arises, this code will still have to be refactored, because the type of the instance is hard-coded. Sure, there could be a factory method instead of NEW zcl_table_zparameters( ). line, but in this case what are the parameters for the factory method? Maybe none? Maybe some? What if you need additional parameters in the future?

On the other hand - do you really need this factory method and all different implementations? Sure, there might be a need in the future, but my guess is this one implementation will be enough for quite some time, as was the original ZPARAMETERS table and simple SELECT from DB, so why overkill the solution with something that won't be used?

2. I did not say that exceptions are bad overall 🙂 I use them too. But in this case, and we seem to agree on this, record not found in DB is not really an error, it's something that may occur as a normal thing, hence the exception in my opinion is a bit too much.
But all in all exceptions are a good thing and much needed in object oriented language.

TL;DR: I'm arguing that using interfaces just because they're there is not the best way. Sure, there are situations where interfaces are great, but I also think that simple inheritance will do just fine in many cases, as will a standalone (no interfaces no inheritance) class. Same goes for exceptions. It's all a matter of common sense and depends on the situation (to not bring the cannon against a mosquito).

BTW: I use the same naming conventions. If you see any other in my answers/comments, then their copied from the original post I'm answering to.

Kind regards,

Mateusz

[matt]

If you inject the actual class, there's no refactoring at all (of the class using the interface). But it really is a matter of philosophy - so long as all the quality factors are up in the 90%, it's good enough!

[matt]

1. Is the interface even needed for what I'm doing here - esp. as it's highly unlikely that there'll be unit tests for the logic? The main reason I created it, is that this seems to be the recommended way for public classes/methods.

The point of having an interface is that you can inject a different implementation into code that uses the interface. It decouples the implementation of a class from the use of that class. But really, it comes down to philosophy. If you always code with interface and classes, then you'll not go wrong.

It has happened to me often enough that "highly unlikely" has become "oh look - it just happened"!

If you do it via interface and find later you need to write a test, you don't have to refactor your code, so you are future proofing it without adding (in my view) complexity.

You can simplify:

DATA int_zparameters TYPE REF TO zif_table_zparameters_dao.
int_zparameters = NEW zcl_table_zparameters( ).

or

DATA(int_zparameters) = CAST zif_table_zparameters_dao( NEW zcl_table_zparameters( ) ).

2. Does the naming for the different entities make sense?

I would not use prefixes for the object references. Especially if you get rid of the cls_ variable as above. For the type, I would use ty_. I find this useful in eclipse with code completion.

3. How to let the calling routine know that no entry might have been found in RETURN_VALUE2 in a functional method only returning exactly one value, in this case the content of VALUE2? At the moment I get around that by querying if VALUE2 is filled but I'm not sure if that't the best/proper way to do it.

There's a few ways

1. Use an exception class

TRY.
  DATA(value2) = int_zparameters->return_value2( VALUE l_zparameters( 
                                                         modul     = 'SD'
                                                         z_program = 'ZPROG'
                                                         field     = 'FIELD'
                                                         value1    = 'VALUE1'
                                                     ) ).

  " Do stuff with value2
  ...
CATCH zx_my_exception INTO data(error).
  WRITE 'No value found'.
ENDTRY.

2. Use an importing parameter and return abap_bool

IF int_zparameters->return_Values( ... IMPORTING e_value2 = value2 ).
  " Do stuff with value2
ENDIF.

3. In the implementation of zcl_table_zparameters make sure that your methods only hit the database one - i.e. buffer the results.

You don't need to clear returning parameters. You might also consider using r_result for any functional method. When eclipse generates a class for you, it defaults to r_result. It is one of the clean code recommendations.

CLASS zcl_table_zparameters DEFINITION  
PUBLIC CREATE PUBLIC.
  PUBLIC SECTION.
    INTERFACES zif_table_zparameters_dao.
  PROTECTED SECTION.
  PRIVATE SECTION.
    DATA:
      db_read TYPE abap_bool,
      entry_found TYPE abap_bool,
      value2 TYPE ...
ENDCLASS.

CLASS zcl_table_zparameters IMPLEMENTATION.
  METHOD zif_table_zparameters_dao~entry_exists.
    IF db_read EQ abap_false.
      read_db( )
     ENDIF.
     r_result = entry_found.
  ENDMETHOD.

  METHOD zif_table_zparameters_dao~return_value2.
    IF db_read EQ abap_false.
      read_db( )
     ENDIF.
     r_result = value2.
  ENDMETHOD.

  METHOD read_db.
      SELECT SINGLE @abap_true, value2
         FROM zparameters
         INTO @entry_found, @value2
        WHERE modul        EQ @i_zparameters-modul
          AND z_program    EQ @i_zparameters-z_program
          AND field        EQ @i_zparameters-field
          AND value1       EQ @i_zparameters-value1
          AND value2       EQ @i_zparameters-value2.
  ENDMETHOD.
ENDCLASS.

TL;DR - pretty sane!

[BaerbelWinkler]

matthew.billingham

Thanks, Matt!

Your comments regarding streamlining/shortening the definitions are helpful (I wonder how long it'll take until I really remember how to do that?!?).

Unfortunately, I don't think that your code example for class zcl_table_parameters will work as I cannot really use the same SELECT statement for what I now have in two methods. When I want to get VALUE2 (because it's e.g. needed to fill another field in the calling routine) I'll not be able to provide that as a WHERE-clause, but I need to provide it if I want to know if an entry exists in the table.

I have to admit that I'm hesitant to add an exception class to the mix as that would require more involved coding in the calling routines and lots of places where these SELECTs are now directly in the code. Not to mention that I thus far don't really understand them and how to use them properly!

Cheers

Bärbel

[matt]

From a philosophical perspective, having separate methods is, I think, best. But pragmatically, it might not be the best solution.

Exceptions are pretty cool, but not the right solution in this case... unless "not found" is a critical error condition. I've been doing OO for ages and still haven't really figured out the best way to use exceptions.

[Sandra_Rossi]

After reading answers of Mateusz and Matthew, I think of another solution for returning a line or null, by using a reference:

METHODS
    return_value2
      IMPORTING
        i_zparameters TYPE zparameters
      RETURNING
        VALUE(result) TYPE REF TO zparameters-value2. " <==== see REF TO here


DATA(value2) = int_zparameters->return_value2( VALUE l_zparameters( 
                                                       modul     = 'SD'
                                                       z_program = 'ZPROG'
                                                       field     = 'FIELD'
                                                       value1    = 'VALUE1'
                                                   ) ).
IF value2 IS BOUND.        " <==== see IS BOUND here
  "Use VALUE2->* as needed   <==== see ->* here
ENDIF.

[Sandra_Rossi]

Missing code:

  METHOD zif_table_zparameters_dao~return_value2.
    SELECT SINGLE value2
         FROM zparameters
         INTO @DATA(value2)          " <==== temporary variable
        WHERE modul        EQ @i_zparameters-modul
          AND z_program    EQ @i_zparameters-z_program
          AND field        EQ @i_zparameters-field
          AND value1       EQ @i_zparameters-value1.
    IF sy-subrc = 0.
      result = NEW #( value2 ).      " <==== needed
    ENDIF.

[egor_malov]

HI, Bärbel,

1. Is the interface even needed

I would keep the interface for the following reasons:

1.1. If you once happen to unit test (which is unlikely now), interface will give you the option to inject some test double object.

1.2. What if one day you realize your new parameter depends on days of the week or phases of the moon, so the parameters table needs more dimensions, so you decide to create another table zparameters2 and transfer older parameter data from zparameters to zparameters2. Interface will allow a safe transition.

2. Naming..

I would probably remove 'table' from the interface\class names, because it is a detail of internal (from the client's point of view) implementation of data persistance.

3. How to let the calling routine know..

I think matthew.billingham's answer is great. I would add that Matthew's option 2 (returning a boolean result) might be preferable because the absence of some parameter is not likely an error, and they say, exceptions should be used for errors only https://github.com/SAP/styleguides/blob/master/clean-abap/CleanABAP.md#exceptions-are-for-errors-not.... So, no exception for this situation is ok.

4 Anything else

4.1 (method entry_exists) Judging from the field names, I guess that modul, zprogram, field are 'dimensions' and value1, value2 are 'values'. So, an entry in this case is a record uniquely identified by modul, zprogram and field. So i think I would expect entry_exists method to be about only these three fields and no valueX fields (neither among imported parameters, nor in the code).

4.2. (method return_value2) I think, it is important to check if select was successful and let the calling code know. (You will get this done when you choose and implement the right approach for question 3).

4.3. Once you have solved question 3, you might not need method entry_exists anymore (just a quick idea)

[BaerbelWinkler]

egor.malov

Thanks, Egor!

As far as the field names of the parameter table goes, they are just that: "names"! Key fields are MODUL, Z_PROGRAM, FIELD and a (manually filled) COUNTER, but the fields VALUE1 and VALUE2 are regularly used to completely qualify an entry in order to determine if processing should proceed. VALUE1 is often an org-unit like sales-org and VALUE2 could be something like order-type, but there's a big variety of how the fields are used. It goes as far as VALUE1 getting queried as a concatenation from more than one field if VALUE1 and VALUE2 are not enough to completely qualify an entry. It's basically a "free for all"!

[egor_malov]

Bärbel,

Thank you for these additional details. This logic sounds a bit more complex than I first thought (well, at least it is definitely beyond some basic name=value parameter store), so two more comments:

1. complexity -> changes are more likely. I think it is another argument in favor of the interface.

2. It might be good to lock this complexity inside the class. So that users of this code wouldn't have to keep in mind complex rules rules like ' sales_org should be passed as value1 and not the other way!'. To do this we could (1) remove the zparameters structure /table from the parameters because it holds all the complexity (2) instead provide a list of parameters that expressly describes what possible parameters are.

before

interface zif_table_zparameters_dao
  public .

  METHODS:
...

    return_value2
      IMPORTING
        i_zparameters TYPE zparameters
      RETURNING
        VALUE(result) TYPE zparameters-value2.

endinterface.<br>

after

interface zif_table_zparameters_dao
  public .
  METHODS:
...
    return_value2
      IMPORTING
       i_module type ??
       i_program type programm
       i_field type fieldname
       i_org_unit type orgeh  optional
       i_sales_org type ???   optional   
      RETURNING
        VALUE(result) TYPE type_of_value2.
endinterface.

[BaerbelWinkler]

egor.malov

Thanks for the additional suggestion, Egor!

The way this parameter table gets used is defined "between" the developer and the IT consultant who "translates" the business requirements into the technical requirements. The two of them basically agree what the values for the different fields are going to be and the developer puts that into the code while the IT consultant fills the table entries in the various systems. This is not a table maintained by end users but only by people in IT. For it to work, it doesn't for example matter if the value used is the sales-org or the company code or something else as long as it's e.g "4711". It could just as well be a made-up value as long as the code and table entry use the agreed value. Is this "pretty"? No, of course not, but it's very flexible and the alternative would be to have a different parameter table for each situation even if it then only has a few entries which wouldn't really help either I think.

[BaerbelWinkler]

sandra.rossi

Thanks, Sandra! Don't your 2nd and 3rd comments qualify as an answer to my question? Might give them more visibility and I find your introduction of a reference variable interesting to say the least!

[Sandra_Rossi]

Here are my comments converted into an answer.

-

1) Is the interface even needed for what I'm doing here - esp. as it's highly unlikely that there'll be unit tests for the logic? The main reason I created it, is that this seems to be the recommended way for public classes/methods.

I wouldn't abstract if there's no reason to do it. Recommendations exist only for good reasons and I don't think it applies to your case.

-

2) Does the naming for the different entities make sense?

If you have a good reason to use an interface, I would rename DAO to DB (I guess DB is better known than DAO), and the instance should also retain DB: db_zparameters. NB: I'm not fan of prefixed Hungarian notation.

-

3) How to let the calling routine know that no entry might have been found in RETURN_VALUE2 in a functional method only returning exactly one value, in this case the content of VALUE2? At the moment I get around that by querying if VALUE2 is filled but I'm not sure if that't the best/proper way to do it.

For returning the information found/not found, it's up to you.

a) Using the initial value doesn't bother me, accompanied with a comment

b) Using an exception could be valid too

c) Using a reference, so you return either some data, or NULL meaning not found in your case.

Definition:

METHODS
    return_value2
      IMPORTING
        i_zparameters TYPE zparameters
      RETURNING
        VALUE(result) TYPE REF TO zparameters-value2. " <==== REF TO

Implementation:

  METHOD zif_table_zparameters_dao~return_value2.
    SELECT SINGLE value2
         FROM zparameters
         INTO @DATA(value2)          " <==== temporary variable
        WHERE modul        EQ @i_zparameters-modul
          AND z_program    EQ @i_zparameters-z_program
          AND field        EQ @i_zparameters-field
          AND value1       EQ @i_zparameters-value1.
    IF sy-subrc = 0.
      result = NEW #( value2 ).      " <==== needed to avoid exception FREED
    ENDIF.
  ENDMETHOD.

Usage:

DATA(value2) = int_zparameters->return_value2( VALUE l_zparameters( 
                                                       modul     = 'SD'
                                                       z_program = 'ZPROG'
                                                       field     = 'FIELD'
                                                       value1    = 'VALUE1'
                                                   ) ).
IF value2 IS BOUND.        " <==== IS BOUND
  "Use VALUE2->* as needed   <==== ->*
ENDIF.

[BaerbelWinkler]

sandra.rossi

Thanks, Sandra! I just now changed my code to using a reference variable as you suggested and it works like a charm!

[matt]

Nice! I'd never thought of doing it like that.

[BaerbelWinkler]

sandra.rossi, matthew.billingham, mateuszadamus, egor.malov

Thanks all for your feedback! I picked Sandra's response as the best answer because I really like and therefore now use her suggestion to work with a reference in order to distinguish between something found (even if "space") and "no entry found".

I also updated my code with some of the other suggestions like not clearing the returning parameters but I kept the interface as I already had it and as your responses were 2 for and 2 (somewhat) against using one in this case.

Before I posed my question on Q&A I wasn't sure if it was a good idea or not to do so, but this resulting discussion with lots of information and food for thought answered that question for me as well: it was definitely worth it!

Cheers

Bärbel

To wrap this up, here is how the code now looks like (not all that different from what I started with):

Interface definition:

interface ZIF_TABLE_ZPARAMETERS_DAO
  public .
  METHODS:
    entry_exists
      IMPORTING
        i_zparameters        TYPE zparameters
      RETURNING
        VALUE(entry_found)   TYPE abap_bool,

    return_value2
      IMPORTING
        i_zparameters        TYPE zparameters
      RETURNING
        VALUE(result)        TYPE REF TO zparameters-value2.
endinterface.

Class:

CLASS zcl_table_zparameters DEFINITION
  PUBLIC CREATE PUBLIC.
  PUBLIC SECTION.
    INTERFACES ZIF_TABLE_ZPARAMETERS_DAO.
  PROTECTED SECTION.
  PRIVATE SECTION.
ENDCLASS.

CLASS zcl_table_zparameters IMPLEMENTATION.
  METHOD zif_table_zparameters_dao~entry_exists.
    SELECT SINGLE @abap_true
         FROM zparameters
         INTO @entry_found
        WHERE modul        EQ @i_zparameters-modul
          AND z_program    EQ @i_zparameters-z_program
          AND field        EQ @i_zparameters-field
          AND value1       EQ @i_zparameters-value1
          AND value2       EQ @i_zparameters-value2.

  ENDMETHOD.

  METHOD zif_table_zparameters_dao~return_value2.
    SELECT SINGLE value2
         FROM zparameters
         INTO @DATA(value2)
        WHERE modul        EQ @i_zparameters-modul
          AND z_program    EQ @i_zparameters-z_program
          AND field        EQ @i_zparameters-field
          AND value1       EQ @i_zparameters-value1.
    IF sy-subrc EQ 0.
      result = NEW #( value2 ).
    ENDIF.
  ENDMETHOD.
ENDCLASS.

Example code to call the methods and use their results:

PARAMETERS: p_exist  TYPE abap_bool AS CHECKBOX,
            p_return TYPE abap_bool AS CHECKBOX,
            p_modul  TYPE zparameters-modul,
            p_prog   TYPE zparameters-z_program,
            p_field  TYPE zparameters-field,
            p_value1 TYPE zparameters-value1,
            p_value2 TYPE zparameters-value2.

"Check if entry exits
DATA:  int_zparameters TYPE REF TO zif_table_zparameters_dao.
int_zparameters = NEW zcl_table_zparameters( ).

TYPES: ty_zparameters  TYPE        zparameters.

IF p_exist EQ abap_true.
  IF int_zparameters->entry_exists( VALUE ty_zparameters( modul   = p_modul
                                                z_program = p_prog
                                                field     = p_field
                                                value1    = p_value1
                                                value2    = p_value2
                                                ) ).
    "Do something when entry exists
  ELSE.
    "Do something else (or nothing) if no entry exists
  ENDIF.
ENDIF.

IF p_return EQ abap_true.

  DATA(value2) = int_zparameters->return_value2( VALUE ty_zparameters( modul = p_modul
                                                       z_program = p_prog
                                                       field     = p_field
                                                       value1    = p_value1
                                             ) ).
  IF value2 IS BOUND.
   "Do something with VALUE2->* 
  ELSE.
   "Do something else if no entry was found
  ENDIF.
ENDIF.

[matt]

If you post a question, I know it will be intelligent and interesting. And, even better, will attract intelligent and interesting answers - I can give my opinion, but I like reading other experienced people's answers, as I am always learning.

[EnnoWulff]

Hi 8b889d0e8e6f4ed39f6c58e35664518f

I don't understand the usage of your routine... You want to check whether there are VALUE1 and VALUE2 maintened in a database table. If yes, you read VALUE2 that you already know...?!

you can return more values than one with RETURNING! But you can only return one parameter. That's true, but the parameter can be a structure.

So you could have something like this:

myval = get_value( .... )-value2.

CLASS main DEFINITION.
  PUBLIC SECTION.

    TYPES: BEGIN OF ts_values,
             one TYPE c LENGTH 2,
             two TYPE c LENGTH 2,
           END OF ts_values.

    CLASS-METHODS get_values
      RETURNING
        VALUE(output) TYPE ts_values.

ENDCLASS.

CLASS main IMPLEMENTATION.
  METHOD get_values.

    output-one = 'AA'.
    output-two = 'BB'.

  ENDMETHOD.

ENDCLASS.

START-OF-SELECTION.

  cl_demo_output=>display_data( main=>get_values( )-two ).

[BaerbelWinkler]

enno.wulff

Hi Enno,

thanks for chiming in and providing another snippet of code!

I actually have two methods, one which just returns if an entry exists for the passed in values and another which retrieves VALUE2 if I know the other values including VALUE1. I may add more methods in the future to e.g. return an internal table with all entries found in the database table fitting the passed in values. Please note that the table is routinely accessed via all kinds of combinations of key- and table-fields and for different purposes as explained in the OP.

Cheers

Bärbel

[EnnoWulff]

Hi 8b889d0e8e6f4ed39f6c58e35664518f I can see the technique, but what is the business case for this? Do you want to check if some "options" or "flags" are set/ active for a special values, e.g. PLANT = 1000?

[BaerbelWinkler]

enno.wulff

Hi Enno,

the table is old and is often used to determine if subsequent statements are to be executed or not. This is usually done via a SELECT SINGLE which comes in all forms or guises and all I want to achieve is to slowly but surely move from these haphazardly coded statements towards the same format via calling the method.

The other functionality is to determine what VALUE2 should be used if there's an entry for the other fields including VALUE1 in the table. The table is basically a "free for all" and gets used in various ways as "negotiated" between the programmer and consultant.

[EnnoWulff]

Thanks for clarifying, 8b889d0e8e6f4ed39f6c58e35664518f . I didn't understand that these are two independent functions.

In this case I would follow the suggestion of mateuszadamus and simplify the call by instantiiating with module and program.

It's confusing because I would assume VALUE1 as PARAMETER and VALUE2 as VALUE. Even if the fields in the corresponding database table are named VALUE1 and VALUE2 I would name them PARAMETER and VALUE in the class.