Help with OO coding (quality and performance)

[former_member201275]

Hi,

I have been coding for some years, but only in procedural abap, not OO. Now I have written an OO program but I have no idea if it is correct, or if I should do it differently, or if there is something I could be doing better.

I have posted below what I have coded. It would be great if an experienced OO person could look at it for me and advise me in terms of quality etc. of how I could improve my OO code. In particular, how I have declared my methods, their visibilty, parameters, etc. You don't need to point out how bad it is as I already know, but assist me in bettering myself.

Hopefully I am not breaking any forum rules, by posting this? If I am please advise me asap and I will of course remove my post immediately. I will reward points for helpful, constructive advice.

To quickly explain what my program does. It simply reads data from a bespoke table to an internal table, and then outputs the internal table to a file on the sap server. I have omitted a lot of my abap code i.e. inner joins and file transfer logic etc., where I know my abap is correct, it is more the OO stuff I would like help with.

Thank you in advance.

Program (SE38):

DATA: gt_9000 LIKE TABLE OF z_extract_9000.
PARAMETERS : p_file LIKE filename-fileintern DEFAULT 'Y_COMP'.

START-OF-SELECTION.
  CALL METHOD zcl_if_9000_extract=>fill_itab_9000

    CHANGING
      pt_9000 = gt_9000.

END-OF-SELECTION.

CALL METHOD zcl_if_9000_extract=>output_file_to_al11
  EXPORTING
    pt_9000 = gt_9000
    pf_file = p_file.

Class (SE24) zcl_if_9000_extract:

FILL_ITAB_9000                Static Method    Public         Populate itab with P9000 data

OUTPUT_FILE_TO_AL11   Static Method    Public         Output File to relevant sap server filepath

GET_FILENAME               Static Method    Protected    Get filename

TRANSFER_FILE              Static Method    Private        Transfer file

Method FILL_ITAB_9000

Parameters:

PT_9000      Changing      Type       ZT_EXTRACT_9000        ITAB for P9000   

METHOD fill_itab_9000.
... code to populate PT_9000
ENDMETHOD.

Method OUTPUT_FILE_TO_AL11

Parameters:

PT_9000       Importing        Type    ZT_EXTRACT_9000              Table for 9000 Extraction

PF_FILE       Importing        Type    FILENAME-FILEINTERN       Logical file name

METHOD output_file_to_al11.
  DATA: g_pfile TYPE pathextern.

  CALL METHOD zcl_if_9000_extract=>get_filename
    EXPORTING
      p_file  = pf_file
    IMPORTING
      g_pfile = g_pfile.

  CALL METHOD zcl_if_9000_extract=>transfer_file
    EXPORTING
      g_pfile = g_pfile
      pt_9000 = pt_9000.
ENDMETHOD.

Method GET_FILENAME

Parameters:

P_FILE      Importing    Type      FILENAME-FILEINTERN       Logical file name

G_PFILE   Exporting    Type      PATHEXTERN                     Physical path name

METHOD get_filename.
  CALL FUNCTION 'ZGET_NAME'
    EXPORTING
      logical_filename = p_file
    IMPORTING
      file_name        = g_pfile.
ENDMETHOD.

Method TRANSFER_FILE

Parameters:

G_PFILE       Importing      Type      PATHEXTERN             Physical path name

PT_9000       Importing      Type      ZT_EXTRACT_9000      Table for 9000 Extraction

METHOD transfer_file.

    LOOP AT pt_9000 INTO ls_9000.

      ASSIGN COMPONENT sy-index OF STRUCTURE ls_9000 TO <fs>.

      CONCATENATE g_data <fs> ';' INTO g_data.
      TRANSFER g_data TO g_pfile.
  ENDLOOP.

ENDMETHOD.

[jrg_wulf]

Hi Glen,

i have to admit, i did no in depth analyzing of what your cod is supposed to do but from what i saw on first glance it's not too easy to find a starting point.

First, this sample code is NOT OO, since it lacks objects.

In OO a class is a general description of behaviour and attributes of similar objects, thus an object is a distinct instance of said class with individual setting of its attributes.

What your code does, is collecting functional routines as class methods. So you're reducing the class to a bag of methods like a funktion-group.

Functional classes, like the one you put in your example, are usually used to provide auxiliary methods, that are not directly related to the attributes of an individual set of data. Or, at least with OO-ABAP, due to the lack of multiple inheritance, methods that are used by many different classes.

So the first step towards OO would be to have instance-methods  and -attributes.

Since i don't know anything about the amount of data handled by your program, it is difficult to make a suggestion towards a better structure.

Perhaps. this isn't the most suitable example to start with, since obviously, you do some data collecting and do a monolithic output.  I don't see any benefit for OO here.

Nevertheless don't let yourself be discouraged. It is still a valuable practice for using OO syntax.

In my experience mass data processing is an area, where procedural programming often remains first choice.

Best regards

Jörg

[SuhaSaha]

In my experience mass data processing is an area, where procedural programming often remains first choice.

Very valid point!

I still have not been able to use Persistence classes in real life coding since they aren't suitable to handle huge data

Maybe someday if i design some UI application i'll put it to use

BR,

Suhas

[former_member201275]

Hi Jörg,

Thank you for your valid points.

I assumed as I had created a class and was calling the methods, this was OO.

My ITAB has 4 fields, and the amount of records I will be processing in this ITAB is around 8000 entries.

Would you recommend I stick with procedural then? If not, then I will create an instance of my class and go from there (i.e. post my code here again for some experienced feedback).

[jrg_wulf]

Hi Glen,

usually i would recommend to stick with procedural in this case. On the other hand, if your code does the trick, there's no harm in it, when coded in OO fashion. As i mentioned before, you can use it to practice the stronger syntax rules in OO context.

The passing of data between report and class methods is nothing slower than it would be with calling of a FM for the same purpose.

Nevertheless, you're welcome to put in a sample of OO code, when you come across something suitable for modeling with classes.

Meanwhile, you could try to get the knack by using existing classes. E.G. give your report an alv output by use of CL_SALV_TABLE or similar.

Regards - Jörg

[rosenberg_eitan]

Hi,

I do not consider my self OO expert I simply try to analyze the current task and then I try to use the best, easiest and chipset way out of it.....

Object-oriented programming is not a way of life but a tool for creating "reusable units of programming logic" (See http://en.wikipedia.org/wiki/Object-oriented_programming).

To use OO just to say "I am using OO" at the pub is a waste of time...

In your case ask yourself is zcl_if_9000_extract is going to be used more then once ?

Regards .

[Former Member]

"I am using OO"

Is that a valid pick-up line at a pub?

[rosenberg_eitan]

I did not try this one yet .

But at my age any trick is appreciated .

Its a pity that I cannot mark your remark as useful.

And maybe after a successful implementation even mark it as the correct one...

[nomssi]

Hello Glen,

I will try to evaluate your code in term of OO analysis and design. Analysis and design are hard. And our knowledge of a problem is most limited while starting to design a solution. I will start from your program description:

It simply reads data from a bespoke table to an internal table, and then outputs the internal table to a file on the sap server.

My understanding of your design is that based on this specification and

your experience in structured programming, you came out with 2 steps to solve the problem.

1. Fill internal table from database
2. Create server file from internal table

This looks like a top down approach where the steps are refined as more details emerge.

To implement this solution you created an ABAP class (I will use a text [CLASS] here):

Class [IF_9000_EXTRACT]:

• Method FILL_ITAB_9000 (query data from DB into table – export parameter - )
• Method OUTPUT_FILE_TO_AL11 – import internal table, logical filename – save data to file

The other methods are not used outside the class and could be private in my view.

Contrast this to my approach. I will try to be verbose to make myself clear.

My aim is to reason about the problem and it solution as much as possible before coding/testing. Using my current experience level in OO analysis, I try to identify OBJECTS that will exchange MESSAGES (method calls) to produce the expected behavior. Let me apply a heuristic to you specification

: objects are nouns, operations are verbs.

• nouns: data, table, internal table, file, sap server
• verbs: Read (from database table) Output (internal table to file)

Now I try to classify and find objects that may help:

1. We are talking about [DATA] that can be READ( ) from a database, and WRITE( ) written to an internal table. And the object DATA can SAVE( ) .. no that sounds awkwards.
2. Maybe new object [FILE] can SAVE( ) to server... that sound better, but where are the DATA? Let us try: Object FILE takes DATA as parameter and SAVE( ) them to the server.
3. Nice, but, what about the server? Is it an object? No Object FILE is initialized with a FileName on a server?

At this point, I usually use paper an pen and start drawing diagrams. Not nice looking/correct UML: it has to start simple and make sense. I start with boxes with link between them and try to find names for the links and it might evolve to class diagrams (with parts: class name, attributes and methods).

I guess I have improved because I now often draw sequence diagrams, but the main point here is to stimulate thinking about the data and INTERFACES. What belong together?  no matter whether  novice or seasoned, you have to decide about WHERE to implement a method, i.e. you have to ASSIGN responsablities.

After doing this for a while, the result will be objects and relations between the objects. I confess I did not spend very long on your design, but I still have confidence the following meets your specification.

Class [EXTRACT]: - I don't know much about the domain, so I choose this name from your code.

• Attribute ITABLE, public read-only attribute. (your global table GT_9000) The data never changes once the object is created
• Private Method QUERY (sounds better than READ) – get data from DB Table into ITABLE.
• Factory method: Creates object, takes parameters and calls QUERY

Objects of this class are created once and do never change. If you need a new set of data, you create a new object.

Class [FILE]:

• Attribute FNAME – FileName on the server, The name never changes while the object is created
• Factory method: takes a parameter and set the FNAME attribute
• Public Method SAVE takes a DATA object an save it to file (your “Output to AL11”)

I have designed to classes so to be able to write the following program:

DATA lo_data TYPE REF TO [EXTRACT].

DATA lo_file TYPE REF TO [FILE].  

lo_data = [EXTRACT]=>factory( ).  

lo_file = [FILE]=>factory( p_file).  

lo_file->save( lo_data ).

I hope anyone can look at the resulting code and find it easy to read. If you agree, then realize that it is by design. I have choosen my objects' interface to make the code read like my understanding of the solution. You can still map messages (Method call) to Step 1) and Step 2), but the objects themselves should also be simple and easy to understand in isolation.

OOA has forced me to reflect more than the steps (procedures) needed to solve my problem. I had to think about single responsibility and the minimal public interface for my objects. I could easily stop the analysis by creating one object [EXTRACT] with method READ( ) and OUTPUT( ) representing step 1) and 2). But I feel that it yields imprudent modelling decisions while coding.

Analysis and design are hard. I am learning, and I have found OO to let you better express your current design in code, no matter at which level you are. The design might be overly complex or not have flexibility enough, but I think OO forces to look at the reality take a decision to accept it as is.

The main task is to improve the understanding of the problem, the model will reflect our level of understanding.

best regards,

JNN

P.S.: If you are looking for guidance, start with web references about SOLID and DRY and read some books. Check the links in Paul Hardy series of blogs.