Technical Articles
example of how to add semantics to a SELECT
Dear community, this week I would like to share another example with you. It’s about my attempts to avoid unnecessary comments. The source code should “tell” the story 🙂 Unfortunately, my source code from previous days doesn’t always tell me why I wrote it. Sometimes it’s really like a puzzle. It annoys me because it steals my time. So I used an example to think about how I can do this better in the future. Here is a piece from a large puzzle. It remains exciting.
<Please note that in the meantime an improved example has been published with this blog.>
In the beginning my source code looked like this.
SELECT SINGLE *
INTO @DATA(usr01)
FROM usr01
WHERE bname = @sy-uname.In my opinion, the SELECT doesn’t “tell” much. You have to pay attention to the following use of structure USR01 to recognize the context (not shown here). But I want to avoid that because it’s unnecessary effort. So I reworked the SELECT statement.
SELECT SINGLE spld, spdb, spda
INTO @DATA(usr01)
FROM usr01
WHERE bname = @sy-uname.Three fields of database table USR01 are of interest. That’s helpful. However, the four-characters, technical names don’t reveal much. What is the meaning of SPLD, SPDB und SPDA? If that were the $ 1 million question in a quiz show, I would have to give up 🙂
Ok, next try (refactoring … please wait …).
SELECT SINGLE spld AS printer,
spdb AS output_immediately,
spda AS delete_from_spool
INTO @DATA(user_print_settings)
FROM usr01
WHERE bname = @sy-uname.I was happy with my last attempt. You can see that it’s about print settings. The INTO statement reveals it additionally. If you know that database table USR01 is there to store user master data, then you could imagine that it’s about the user specified print settings. Who would have thought that?
I implemented the source code in a method called GET_USER_PRINT_SETTINGS. This makes it even more clear to me.
Any suggestions to make that better? I know that may seem exaggerated. But I want to create more readable and maintainable source code in the future 🙂
<Please note that in the meantime an improved example has been published with this blog.>
Best regards, thanks for reading and please stay healthy
Michael
P.S.: Please support the virtual wishing well.
P.S.S.: Not tired of reading blogs? Check this blog by Marcello Urbani.
Spot on! This is the way to do it.
Thanks for this blog post; I've rarely seen one that was so much in line with one of my own pet peeves.
"Refactor towards understanding" is exactly what I try to practice (and teach).
In fact, it was the main theme of my recent presentation at #sitMel (SAP inside track Melbourne).
If you're interested, you can find links to the video recording and the presentation material here https://lbreddemann.org/sap-inside-track-sitmel-full-day-event-in-melbourne/.
In the presentation I included two, let's say, more involved examples of how refactoring SQL can lead to more obvious code and offer better options to improve performance.
Cheers,
Lars
Time 20:47 in the record: "This is DBA country!" ... Best allusion in a long time 🙂
Glad someone got it (and liked it ?)
seems to be a generational thing ...
I have a special sense of humor. More people should quote films. This is how you get to know new films.
Hello Michael,
It does not seem exaggerated to me. Ok, next try 🙂
Why is it called
if there is no USER in the structure?
I would add a user column to the select.
best regards,
JNN
Accepted 🙂
Or just remove the user from the name. That's probably all the caller is interested in too since it would already know the user name. I also like to try use English grammar for method names if I can:
First decide which object the method belongs to (single responsibility).
If it is a LCL_USER object, than the settings a retrieved in a context where we have the user name as attribute. You do not need user in the structure or in the method name. Your proposed method name makes more sense in a controller object, e.g. class LCL_APP. We should then have/keep the user name in the structure.
Yes it depends on the context. I think if the caller consumes the result and doesn’t need or use the username after the call (e.g. if it’s preparing some output) then it’s irrelevant in the data structure. It may be relevant e.g. if the caller determines a username, calls the print settings getter and then returns the combined result back higher up. Otherwise it seems redundant to me to return the same data that we pass in.
However I think the method name has a different reason to include “user”, my example was just to show the use of grammar to Englishify it, to write it closer to how we would talk.
get_print_settings_for_user( sy-uname )
reads nicer than
get_print_settings( sy-uname )
regardless of whether we get a username back in the result. It just documents the parameter.
Do not forget the objects. It is more like
DATA(print_settings) = lo_user->get_print_settings( )
or
lcl_app=>get_print_settings_for_user( sy-uname ).
The name of the object is part of the call and documentation.
True 🙂
I like this! Readable code. No need to try to figure out what a strange variable is. No more guess work on what I or another programmer was thinking. No comment needed. Have I said enough to let you know I really like it when people do this in their code? ?
Oh no, we will no longer have fun guessing semantics 😉
Hi Michael,
I agree, this is much more informative than where it started. Another thing that is helpful is to pull all SQL statements out into their own class (or classes, depending on the complexity of the system). Then the method name can provide further context of what the SQL is actually doing. I use 1 method per SQL statement, where the method name always starts with the SQL verb (SELECT, INSERT, etc.) and then rest of the name provides the context of what is being selected, inserted, etc.
Self-documenting code is a wonderful thing!
-Scott
Very good hint. The class is the framework. A bit like in the garden with the beds. Carrots over here, salad over there.
What if you want to make a mixed salad? 🙂
Mixed salad is a composition: CL_MIXED_SALAD makes use of CL_CARROTS and CL_SALAD 😉
Thank you, Michael. That is an excellent solution. I really never considered doing it this way, but I will start adding this to my code immediately.
Same situation here. I wish I knew that before. Years of code that tell no story. How boring is that?
Simple, elegant. I like it.
Quite a simple way to improve the readability of code. Specifically talking, my code! 😛
I will definitely be using this. There are some great suggestions in the comments as well!
Great blog post, appreciate you sharing it!
My pleasure. That being said, I also read a lot of source code from other developers. I like it when the code quickly makes it clear what is meant. Therefore, sharing such small possibilities makes a lot of sense to me 😉
Very nice!
A teacher once told me "Der Code spricht!" => "The code speaks!"(...for itself).
Yours does indeed!
Hopefully he doesn't speak in riddles 😉
You can also consider to use
instead of SY-UNAME to be really sure to have the user name of the logged on user.
I am not sure if this is really necessary, because if you cannot rely on the SY fields in your program, then there is something wrong. If a user has the power to change field values in the debugger, he/ she will have a reason to do so...
Great post, super useful and adding the idea to our developer guidelines.