I wrote about the need for an 10 years old ABAP developer to update programming skills in order to align them with the market’s current requirements, an important aspect in everyone’s career, including both in-house and freelance consultants. Then I introduced the example of a guitar shop owner named Mike who needs an app within SAP to keep tracking of his instruments. Silly example since everything could be done with the standard, I know, but a good exercise to put programming concepts into practice.
After succesfully passing the two tests implemented for the add_guitar() method in the inventory class, the next step is to implement a search() method.
To remember, this is the original class diagram
After succesfully passing the two tests implemented for the add_guitar() method in the inventory class, the next step is to implement a search() method.
To remember, this is the original class diagram
Folowwing orthodox TDD, before coding the actual search() method, we implement a failed test, then write just enough code to pass the test. So we need to add a search_existing_guitar( ) method within the test class
method search_existing_guitar.
* First create some test guitars to add to inventory
initialize_inventory( ).
* Then instantiate the guitar object to search
data(attrib_search) = value zguitars( builder = 'Fender'
model = 'Stratocaster'
type = 'electric'
backwood = 'Alder'
topwood = 'Alder' ).
data(guitar_to_search) = new zcl_guitar( attrib_search ).
* Search the sample guitar into the inventory
data(found_guitar) = inventory->search( guitar_to_search ).
* Finally, determine whether the guitar was found
cl_abap_unit_assert=>assert_bound( act = found_guitar " Reference variable to be checked
msg = 'Guitar was not found' ).
endmethod.
Initialize_inventory is a helper method (not a test method) which adds guitars to the inventory using the add_guitar method.
method initialize_inventory.
data(guitar_to_add) = value zguitars( serialnumber = '11277'
price = '3999.95'
builder = 'Collings'
model = 'CJ'
type = 'acoustic'
backwood = 'Indian Rosewood'
topwood = 'Sitka' ).
data(guitar) = new zcl_guitar( guitar_to_add ).
inventory->add_guitar( guitar ).
guitar_to_add = value zguitars( serialnumber = 'V95693'
price = '1499.95'
builder = 'Fender'
model = 'Stratocaster'
type = 'electric'
backwood = 'Alder'
topwood = 'Alder' ).
guitar = new zcl_guitar( guitar_to_add ).
inventory->add_guitar( guitar ).
guitar_to_add = value zguitars( serialnumber = 'V9512'
price = '1549.95'
builder = 'Fender'
model = 'Stratocaster'
type = 'electric'
backwood = 'Alder'
topwood = 'Alder' ).
guitar = new zcl_guitar( guitar_to_add ).
inventory->add_guitar( guitar ).
guitar_to_add = value zguitars( serialnumber = '122784'
price = '5495.95'
builder = 'Martin'
model = 'D-18'
type = 'acoustic'
backwood = 'Brazilian Rosewood'
topwood = 'Adriondack' ).
guitar = new zcl_guitar( guitar_to_add ).
inventory->add_guitar( guitar ).
guitar_to_add = value zguitars( serialnumber = '76531'
price = '6295.95'
builder = 'Martin'
model = 'OM-28'
type = 'acoustic'
backwood = 'Brazilian Rosewood'
topwood = 'Adriondack' ).
guitar = new zcl_guitar( guitar_to_add ).
inventory->add_guitar( guitar ).
guitar_to_add = value zguitars( serialnumber = '70108276'
price = '2295.95'
builder = 'Gibson'
model = 'Les Paul'
type = 'electric'
backwood = 'Mahogany'
topwood = 'Maple' ).
guitar = new zcl_guitar( guitar_to_add ).
inventory->add_guitar( guitar ).
guitar_to_add = value zguitars( serialnumber = '82765501'
price = '1890.95'
builder = 'Gibson'
model = 'SG 61 Reissue'
type = 'electric'
backwood = 'Mahogany'
topwood = 'Mahogany' ).
guitar = new zcl_guitar( guitar_to_add ).
inventory->add_guitar( guitar ).
guitar_to_add = value zguitars( serialnumber = '629584'
price = '2100.95'
builder = 'PRS'
model = 'Dave Navarro Signature'
type = 'electric'
backwood = 'Mahogany'
topwood = 'Maple' ).
guitar = new zcl_guitar( guitar_to_add ).
inventory->add_guitar( guitar ).
endmethod.
After running the test, of course it does not find anything, since the search method is empty
Let’s create a simple linear search algorithm that takes in a guitar object reference, and return the found object. Nothing complicated, just iterate through all guitars, and compare each of its properties.
"! <p class="shorttext synchronized" lang="en"></p>
"! Iterates through the inventory, comparing each property of the i_guitar object with the guitar
"! inside the inventory. Returns a guitar object only if all of the properties match
"! @parameter i_guitar_to_search | <p class="shorttext synchronized" lang="en"> Guitar to search</p>
"! @parameter r_found_guitars | <p class="shorttext synchronized" lang="en">Guitar found in the inventory</p>
methods search
importing
i_guitar_to_search type ref to zcl_guitar
returning
value(r_found_guitar) type ref to zcl_guitar.
method search.
loop at guitars assigning field-symbol(<guitar>).
if <guitar>-ref->attributes-builder = i_guitar_to_search->attributes-builder
and <guitar>-ref->attributes-model = i_guitar_to_search->attributes-model
and <guitar>-ref->attributes-type = i_guitar_to_search->attributes-type
and <guitar>-ref->attributes-backwood = i_guitar_to_search->attributes-backwood
and <guitar>-ref->attributes-topwood = i_guitar_to_search->attributes-topwood.
r_found_guitar = <guitar>-ref.
return.
endif.
endloop.
endmethod.
Run the test again and it passes
Now, I have many problems with this implementation. First, these string comparissons are not really efficient. It only takes a typo, or a incorrect capitalized letter for the test to fail. Second, what if the owner wants his clients to have multiple choices? This search method only returns one guitar, that certainly won’t make him happy. Third, the ZCL_INVENTORY and ZCL_GUITAR classes depend too much on each other.
With so many things to fix, it can be hard to know where to start. Now, this is an oversimplification of a real world problem, but i’m pretty sure you can relate to this story, and have been in the same situation. Too many things to fix, time is too short, customer wants to see results as soon as possible.
Where do we start then? In the book head first Object Oriented Analysis and Design (OOA&D) there is a guide on the steps to take in order to build great software. So simple, yet so powerful.
- Make sure your application does what your user -or customer- wants it to do.
- Apply basic Object-Oriented principles to add fleixiblity.
- Strive for maintainable, reusable design.
That’s it, by following these basic principles we can make sure that we have written great code. In my experience most developers stop at the end of the first phase, then start working in the next requirement, many times following instructions given by their superiors -This is why I don’t fancy software factories created by big consulting companies.
Applying this process to our example, let’s start with functionality: What is going with that search() method anyways? Those string comparisons are annoying and fragile, let’s avoid them completely. What can be a good replacement? Enumerated types can be a good candidate.
I’m using the SAP NetWeaver AS ABAP and SAP BW 7.5 SP01 on SAP HANA SP10 [Developer Edition] available in the SAP cloud appliance, unfortunately it won’t be possible to use the new built-in enums which are available for ABAP as of NetWeaver 7.51.
Nevertheless, I found a good approach available in the standard, contained in the package S_BCFG_API. Kudos to the guy from SAP who coded this, it’s a great workaround if you want to use enums in a pre-751 system (99.9% of all customers right now).
Inside this package there are some standard classes which implement enums, the base class being CL_BCFG_ENUM_BASE. So I made a copy from this base class, and created one enumeration class for each of these string properties (Guitar type, Builder, Wood).
The base class looks like this:
class zcl_enum_base definition public abstract create protected.
public section.
methods get_id final returning value(result) type string.
protected section.
types:
t_enum_values type hashed table of ref to zcl_enum_base with unique key table_line.
class-methods: value_by_id importing i_type_name type string
i_id type string
returning value(result) type ref to zcl_enum_base,
values_by_name importing i_type_name type string
returning value(result) type t_enum_values.
methods constructor importing i_id type string.
private section.
types: begin of s_enum,
id type string,
ref type ref to zcl_enum_base,
end of s_enum,
t_enums type hashed table of s_enum with unique key id,
begin of s_enums_by_type_name,
type_name type string,
enums type t_enums,
end of s_enums_by_type_name,
t_enums_by_type_names type hashed table of s_enums_by_type_name
with unique key type_name.
class-data: all_enums type t_enums_by_type_names.
data: id type string.
endclass.
class zcl_enum_base implementation.
method get_id.
result = to_upper( me->id ).
endmethod.
method constructor.
field-symbols:
<enums_by_tn> type s_enums_by_type_name.
data:
enums_by_tn like <enums_by_tn>,
enum like line of <enums_by_tn>-enums,
type_name type string.
me->id = i_id.
" store newly created enum in enum data model
enum-id = me->id.
enum-ref = me.
type_name = CAST cl_abap_typedescr( cl_abap_datadescr=>describe_by_object_ref( me ) )->get_relative_name( ).
read table zcl_enum_base=>all_enums with table key type_name = type_name assigning <enums_by_tn>.
if ( <enums_by_tn> is not assigned ).
enums_by_tn-type_name = type_name.
insert enums_by_tn into table zcl_enum_base=>all_enums.
assert sy-subrc = 0. " expected by this framework
read table zcl_enum_base=>all_enums with table key type_name = type_name assigning <enums_by_tn>.
assert sy-subrc = 0. " expected by this framework
endif.
insert enum into table <enums_by_tn>-enums.
assert sy-subrc = 0. " expected by this framework
endmethod.
method values_by_name.
read table zcl_enum_base=>all_enums with table key type_name = i_type_name into data(enums).
assert sy-subrc = 0. " it's ensured by framework/pattern
loop at enums-enums into data(enum).
insert enum-ref into table result.
endloop.
endmethod.
method value_by_id.
data: id type string.
read table zcl_enum_base=>all_enums with table key type_name = i_type_name into data(enums).
assert sy-subrc = 0.
id = to_upper( i_id ).
read table enums-enums with table key id = id into data(enum).
if sy-subrc <> 0.
"Raise exception unsupported
endif.
result = enum-ref.
endmethod.
endclass.
Later on, all of your enumerated types will be subclasses of this one. For example, to create a guitar_type enum to replace the “type” attribute (Acoustic guitar – Electric guitar), I created the class ZCL_ENUM_GUIT_TYPE as follows:
class zcl_enum_guit_type definition
public
inheriting from zcl_enum_base
final
create private .
public section.
types:
t_enum_guit_type type hashed table of ref to zcl_enum_guit_type with unique key table_line.
class-data:
acoustic type ref to zcl_enum_guit_type read-only,
electric type ref to zcl_enum_guit_type read-only.
class-methods:
class_constructor,
value_of importing i_id type string
returning value(result) type ref to zcl_enum_guit_type,
values returning value(result) type t_enum_guit_type.
protected section.
private section.
constants:
id_acoustic type string value 'ACOUSTIC',
id_electric type string value 'ELECTRIC'.
endclass.
class zcl_enum_guit_type implementation.
method class_constructor.
acoustic = new zcl_enum_guit_type( i_id = id_acoustic ).
electric = new zcl_enum_guit_type( i_id = id_electric ).
endmethod.
method values.
data concrete type ref to zcl_enum_guit_type.
data(enums) = zcl_enum_base=>values_by_name( 'ZCL_ENUM_GUIT_TYPE').
loop at enums into data(enum).
concrete ?= enum.
insert concrete into table result.
endloop.
endmethod.
method value_of.
result ?= zcl_enum_base=>value_by_id( i_type_name = 'ZCL_ENUM_GUIT_TYPE'
i_id = i_id ).
endmethod.
endclass.
This way, instead of refering to a guitar type as “electric” or “acoustic”, I would just say ZCL_ENUM_GUIT_TYPE=>electric or ZCL_ENUM_GUIT_TYPE=>acoustic. What if you still need the string property? You can use the GET_ID( ) method defined in the super class:
TYPES: BEGIN OF elem,
id TYPE i,
type TYPE REF TO zcl_enum_guit_type,
END OF elem.
DATA(elem) = VALUE elem( id = 34
type = zcl_enum_guit_type=>acoustic ).
cl_demo_output=>display( data = elem-type->get_id( ) ).
Output looks like this:
Let’s implement the first change to the search() method. This is fairly simple, just return an internal table of found guitars, instead of just one.
types: begin of ty_guitar,
serial_number type z_serial_number,
ref type ref to zcl_guitar,
end of ty_guitar,
guitars_tab type hashed table of ty_guitar with unique key serial_number.
"! <p class="shorttext synchronized" lang="en"></p>
"! Iterates through the inventory, comparing each property of the i_guitar object with the guitar
"! inside the inventory. Returns a guitar object only if all of the properties match
"! @parameter i_guitar_to_search | <p class="shorttext synchronized" lang="en"> Guitar to search</p>
"! @parameter r_found_guitars | <p class="shorttext synchronized" lang="en">Guitar found in the inventory</p>
methods search
importing
i_guitar_to_search type ref to zcl_guitar
returning
value(r_found_guitars) type guitars_tab.
method search.
loop at guitars assigning field-symbol(<guitar>).
if <guitar>-ref->attributes-builder = i_guitar_to_search->attributes-builder
and <guitar>-ref->attributes-model = i_guitar_to_search->attributes-model
and <guitar>-ref->attributes-type = i_guitar_to_search->attributes-type
and <guitar>-ref->attributes-backwood = i_guitar_to_search->attributes-backwood
and <guitar>-ref->attributes-topwood = i_guitar_to_search->attributes-topwood.
data(found_guitar) = value ty_guitar( serial_number = <guitar>-ref->attributes-serialnumber
ref = <guitar>-ref ).
insert found_guitar into table r_found_guitars.
endif.
endloop.
endmethod.
The second change is to replace the property strings with the corresponding enum classes. So we’re gonna need to update the attributes definition in the ZCL_GUITAR class.
types: begin of ty_guitar_attributes,
serialnumber type z_serial_number,
builder type ref to zcl_enum_builder,
model type z_guitar_model,
type type ref to zcl_enum_guit_type,
backwood type ref to zcl_enum_wood,
topwood type ref to zcl_enum_wood,
price type z_price,
currency type waers,
end of ty_guitar_attributes.
And update the search_existing_guitar() test method.
method search_existing_guitar.
* First create some test guitars to add to inventory
initialize_inventory( ).
* Then instantiate the guitar object to search
data(attrib_search) =
value zcl_guitar=>ty_guitar_attributes( builder = zcl_enum_builder=>fender
model = 'Stratocastor'
type = zcl_enum_guit_type=>electric
backwood = zcl_enum_wood=>alder
topwood = zcl_enum_wood=>alder ).
data(guitar_to_search) = new zcl_guitar( attrib_search ).
* Search the sample guitar into the inventory
data(found_guitars) = inventory->search( guitar_to_search ).
* Finally, determine whether the guitar was found
cl_abap_unit_assert=>assert_not_initial( act = found_guitars " Reference variable to be checked
msg = 'No guitars found' ).
endmethod.
Adding so many guitars manually is really painful. Lots of copy-paste, code looks cluttered. Even after hiding it under a helper method. It turns out that some smart people already thought about this, and created a very neat way of using test data in a much simpler way. But I will leave this for the next post.
I would like to finish with an update of the class diagram, after these changes.
No comments:
Post a Comment