Reports still writing directly to the screen are very hard to maintain whenever changes to the layout are required. On such occasions, review the code and, if the effort involved is not too big, consider converting it to ALV. Always involve the functional people in this decision.
Historically ABAP programs tend to grow very loooong. All programming best practices teach us there is not a single advantage in this approach. If any routine, be it a program, a method, a function or anything else, becomes longer than 200-300 lines, question it and seriously consider refactoring it into several sub-routines. This has the added advantage of potentially increasing code reuse. But the greatest advantage is encapsulation, isolating variables in their local context, instead of having all of them together, resulting in safer and more readable code.
If the same piece of code is repeated at least once, question yourself why and try to avoid it by creating a reusable routine. If there is more than one SELECT for the same table in a program, make sure you can’t merge them into a single one. Sometimes a smart use of RANGEs to unify parameters can avoid the need for multiple SELECTs. If the same code is used in 2 different programs, don’t repeat the code.
The more global variables a program has, the most obscure it becomes. Please avoid them. This is a basic rule of good programming and should always be followed. Even if several variables have to be passed by parameter, it takes slightly more effort but yields a much more readable and safer code. Exceptions can be made for simple reports which run around a single internal table, which can be declared globally without compromising clarity.
It is common to find an ALV data structure explicitly defined in the code. If this is done, the field catalog has to be manually constructed. If a predefined structure (from DDIC or declared as a TYPE) is used instead, the field catalog can be automatically built. This approach is always better and results in less code, even if the field catalog needs to be adjusted here and there. https://abapinho.com/en/2011/12/automatizar-catalogo-alv/
Many times we do READ TABLE itbl or LOOP AT itbl just to do a CHECK SY-SUBRC = 0. In these cases, the actual data read is not needed. For these cases always use TRANSPORTING NO FIELDS. This way is faster and avoids having to declare a target structure.
In classes, consider using exceptions classes over the old ones. These have great advantages and, once understood, are simple and allow for simpler code. https://zevolving.com/2011/12/class-based-exception/
In reports, instead of WRITE TEXT-001, use WRITE ‘bla bla bla’(001). This way a default text will always be there and besides the readability of the program is improved. https://abapinho.com/en/2011/11/programas-poliglotas/
SALV classes are more versatile and more recent than the old function modules. So, for new ALVs always use SALV. The only exception is editable ALVs which SALV classes are still very incapable of doing. https://scn.sap.com/docs/DOC-10365 https://scn.sap.com/docs/DOC-10366
Since data operations are much more optimized in the database server than in ABAP, it is always better to use the first as much as possible. FOR ALL ENTRIES should only be used when INNER JOIN doesn’t give us what we need or is not possible (like with BSEG for example). Artificial ranges are also a possible alternative to FOR ALL ENTRIES but be careful not to reach the SQL parser limit.
User-exits (and enhancements) are usually crowded with CHECKs. The tragic consequence is that, if the check fails, nothing else after it will run. Not even the standard code. Always try to correct these. Encapsulating the code inside a routine (ideally a method) is enough to render it harmless.
When declaring structures which will receive data from an internal table, instead of declaring its type directly, use LIKE LINE OF. This way not only it becomes clear that they are related but also, if you happen to change the type of the internal table, you won’t have to worry about updating the structure’s type.
Never do COMMITs inside user-exits. Also, make sure any external routine you call from there doesn’t do any COMMIT either.
If a development requires more than one customising table consider grouping their maintenance views under a cluster. This way it will be more intuitive to maintain them. This makes even more sense if one depends on the other since in the cluster definition these relations can be explicitly defined. Example: Como encavalitar tabelas (portuguese)
Always try to select only the fields you’ll need. Selecting others is a waste of resources. Exception made for the use of FM *_SINGLE_READ because, even though these do select all fields, since they cache the data, they are still faster when used multiple times for the same key. If you just want to check if a record exists, select just one field, if possible the one you’re using as criteria to avoid declaring an extra variable.