ABAP bad practice

Dzheki-Chan-memПо своему роду деятельности часто приходится разбираться с чужим ABAP кодом, в котором постоянно встречаются одни и те же проблемы, вызывающие «головную боль» при сопровождении. В данной статье будут рассмотрены основные из них.

Незнание основополагающих принципов разработки

Описанные далее принципы не относятся напрямую к языку ABAP и применимы так же к другим языкам.

 

KISS принцип

Существует множество расшифровок данного принципа:

  • Keep it simple, stupid.
  • Keep it small and simple.
  • Keep it sweet and simple.
  • Keep it simple and straightforward.
  • Keep it short and simple.
  • Keep it simple and smart.
  • Keep it strictly simple.

Проще говоря, программный код должен состоять из как можно более простых для понимания и сопровождения блоков. Не должно быть «портянок» по 10 тыс. строк подряд, особенностью такого кода является увеличение вложенности условных конструкций и общее усложнение логики работы. Относительно гайдлайна по ABAP, процедура или метод должны содержать не более 150 выражений.

На приведенном ниже рисунке (из официальной документации ABAP) показан пример того, как после рефакторинга 1 метод со сложной логикой был разделен на 3 отдельных.

pic1

Подобный рефакторинг позволяет упростить для понимания и сопровождения систему. Каждый из выделенных методов отвечает за конкретные операции. В лучшем случае изменяя один, не придется менять остальные.

Пример:

В данном примере можно выделить запись истории в отдельную процедуру, которая будет отвечать именно за запись журнала:

При необходимости изменения таблицы с журналами, вам не придется менять остальные процедуры в которых вызывается процедура записи истории.

 

Разделение ответственности (SoC)

Разделение ответственности (separation of concerns) говорит о разделении программы на функциональные блоки и о том, что эти функциональные блоки не должны перекрывать друг друга относительно своих функций. Взаимодействие между такими блоками должно быть через определенные для этого интерфейсы. Разделение ответственности позволяет снизить системную сложность, повысить надежность и адаптивность программ, а также обеспечить возможность их повторного использования.

В качестве примера рассмотрим стандартный пример из справки ABAP:

В данном примере видно классический образец портянки на её начальной стадии (для столь простых программ как в примере, соблюдение принципа не является необходимостью, однако, когда речь идет больших программах, его нарушение чревато проблемами при сопровождении). Программа состоит из нескольких логических блоков: получения, проверки (обработки) и отображения данных, однако все эти блоки слиты воедино в обработке START-OF-SELECTION (не явно).

Далее приведен пример того, как программа была разделена относительно блоков, отвечающих за конкретные действия с соблюдением принципа:

 

YAGNI

Сокращение от английского (You Ain’t Gonna Need It – вам это не понадобится). Принцип, который декларирует в качестве основной цели отказ от избыточной функциональности или от добавления функциональности, в которой нет необходимости.

Желание написания кода, который может потребоваться в будущем приводит к следующим последствиям:

  • Тратится время, которое было бы затрачено на добавление, тестирование и улучшение необходимой функциональности.
  • Новые функции должны быть отлажены, документированы и сопровождаться.
  • Новая функциональность ограничивает то, что может быть сделано в будущем, — ненужные новые функции могут впоследствии помешать добавить новые нужные.
  • Пока новые функции действительно не нужны, трудно полностью предугадать, что они должны делать, и протестировать их. Если новые функции тщательно не протестированы, они могут неправильно работать, когда впоследствии понадобятся.
  • Это приводит к тому, что программное обеспечение становится более сложным (подчас чрезмерно сложным).
  • Если вся функциональность не документирована, она может так и остаться неизвестной пользователям, но может создать для безопасности пользовательской системы различные риски.
  • Добавление новой функциональности может привести к желанию ещё более новой функциональности, приводя к эффекту «снежного кома».

Пример нарушения, с которым часто приходится сталкиваться:

Программист заранее предоставил функционал, который никогда не будет использован, при этом его разработка потребовала дополнительного времени на тестирование и проверку существующего кода, в коде программы остались блоки, которые требуется тестировать в процессе сопровождения каждый раз, когда программа меняется. По истечении нескольких лет, никто уже и не вспомнит, зачем был написан блок обработки параметра b, т.к. зачастую такой «доп. функционал» никем не документируется.

 

Принцип DRY

От английского Don’t repeat yourself – не повторяй себя. Принцип призывает к уменьшению сложности ПО, путем создания отдельных управляемых компонентов, отвечающих за конкретные функции, без дублирования этих функций между компонентами, впрочем, принцип настаивает на отсутствии дублирования любой информации. Звучит он так: «Каждая часть знания должна иметь единственное, непротиворечивое и авторитетное представления в рамках системы». Когда этот принцип применяется успешно, изменение единственного элемента системы не требует внесения изменений в другие, логически с ним не связанные.

Данный принцип можно отнести как к отдельным блокам ПО таким как: классы, функции, методы, процедуры, так и к элементарным наборам операторов. Каждый блок ПО на низком уровне должен отвечать за свое конкретное назначение и не должен дублироваться, изменение в таком блоке не должны нести за собой изменения в других не связанных с ним блоках. Рассмотрим данный принцип на элементарном уровне (отсутствие дублирования кода):

Как видно из примера, каждый раз, когда мы изменяем таблицу с данными, программист начитывает дополнительный текст к ним. Соблюдая DRY вместо копирования SELECT’a программист должен был вынести данную логику в отдельную процедуру, позаботиться о необходимости кэширования и т.п.

Рассматривая принцип на более высоком уровне можно представить следующую ситуацию: допустим, вы получаете данные из удаленной системы, для этого был реализован специальный класс «А», если данные из этой системы понадобятся вновь, вы не станете разрабатывать (копировать) новый класс «B», а обратитесь к уже существующему «А» (при необходимости расширив его).   Предположим, что вы разработали класс «B» вопреки принципу, в таком случае при изменении логики обращения к удаленной системе, вы должны будете внести изменения в оба класса.

В дополнении отличная статья: Три ключевых принципа ПО, которые вы должны понимать.

Несоблюдение правил разработки в ABAP

Большинство описанных далее ошибок являются знакомыми опытным ABAP программистам, однако даже среди людей, занимающихся разработкой на ABAP не первый год, можно встретить тех, кто их до сих пор допускает.

 

ABAP Objects

Язык ABAP является гибридным языком, в котором одновременно можно использовать как процедурный подход, так и ОО (объектно-ориентированный) подход. Относительно разницы в подходах и преимущества ООП сложено немало статей и книг, однако многие программисты до сих пор не приемлют ООП. Так или иначе, его изучение и использование понадобится, т.к. все новые технологии в SAP тесно связаны с ООП и не могут быть использованы без него. Например, ADBC, AMDP, Persistence Objects, RTTS, ICF, Web Dynpro и многие другие.

Говоря об ABAP Objects нельзя не упомянуть об GRASP и GOF паттернах, применение которых, давно уже считается хорошим тоном, а их несоблюдение ведет к проблемам сопровождения и усложнению общей архитектуры. Данная тема является весьма обширной и в данной статье рассмотрена не будет, как и принципы объектно-ориентированного анализа и проектирования.

 

Устаревшие конструкции

ABAP — динамично развивающийся язык, многие конструкции языка используемые в его ранних версиях в настоящий момент либо запрещены к использованию (Контекст ООП), либо помечены как устаревшие. При написании нового кода следует учитывать это и не использовать такие конструкции.

Пример:

Оператор SEARCH является устаревшим, следующий пример использует новую конструкцию:

Кроме непосредственно операторов, могут устаревать и функции, пример из SLIN:

pic2

Кроме вышеперечисленного могут так же устаревать подходы к использованию того или иного функционала: использование ALV на базе вызовов FM относительно использования объектной модели или использование CL_GUI_ALV_GRID вместо Simple ALV, если нет необходимости функций редактирования. Использование обычной ALV таблицы, вместо оптимизированной для HANA, использование ФМ по отправке почты SO_NEW_DOCUMENT_SEND_API1 вместо CL_BCS. Классическая обработка исключений вместо ООП исключений и многое другое.

 

Пренебрежение проверками

В SAP системе уже давно разработаны системы проверки ABAP кода (тр. SCI, SLIN и другие), которые позволяют в автоматическом режиме увидеть множество проблем в коде: неиспользуемые процедуры и переменные, устаревшие операторы, ошибки в обработке вызовов ФМ и прочее. Однако некоторые программисты все еще ими пренебрегают. В итоге, зайдя в написанную таким программистом программу и выполнив расширенную проверку, мы можем увидеть множество проблем:

pic3

 

Отсутствие форматирования и соблюдения правил наименования

Один из разбираемых мной примеров:

pic4

На рисунке явно видно, что автор не знает, что такое «структурная печать» и как её выполнять, кроме того переменные названы как попало, в итоге не знаешь, что может скрываться за переменной p_per – параметр на экране или может быть диапазон из select-option, а может быть локальная переменная, а может глобальная.  В примере переменная tab_1 объявлена на глобальном уровне программы, однако разбираясь в коде, не сразу до этого доходишь.

Иногда встречаются такие программисты, которые обуславливают невозможность использование структурной печати тем, что сбивается регистр букв, а они используют camel Case в своем коде, чтобы расставить отступы и не сбить регистр, достаточно настроить структурную печать:

pic5

В разных компаниях правила наименования могут отличаться друг от друга, но они вносят порядок в код и повышают его читаемость. В рамках одной компании лучше придерживаться общих правил (если такие определены). Кроме того, в инспекторе кода давно уже есть возможность автоматически проверять соответствие установленным правилам.

К данному пункту можно так же отнести нарушение правила: одно выражение на одну строку кода:

Вместо:

 

Использование не оптимальных конструкций

Элементарные правила написания оптимального кода зачастую не соблюдаются, приходится видеть многократные вложенные циклы SELECT ENDSELECT, либо выборку из БД в циклах по внутренним таблицам, выборку всех данных из таблицы * тогда, когда это не необходимо и т.п.

Относительно оптимизации ABAP кода есть множество правил, многие из них представлены в соответствующей литературе и документации к ABAP:

В настоящее время, при использовании In-memory решений, таких как HANA, некоторые из них пересматриваются: если раньше требовалось как можно меньше нагружать СУБД (парадигма Data-to-Code), то сейчас в системах с HANA основные расчёты с большим объемом данных требуется производить в СУБД (Парадигма Code-to-Data), а на сервер приложений выдавать уже агрегированные и рассчитанные данные.

 

Разбиение логики работы кода относительно систем

Логика работы программы должна быть одинакова во всех системах, будь то тестовая система или продуктивная, однако часто можно встретить логические условия относительно имени системы, которые просто меняют поведение программы. Как в таком случае вносить изменения и проводить тестирование не совсем ясно, часто встречаешь в таком случае ответ: «проверим в продуктивной системе», зачем в таком случае тестовая система и система разработки остается загадкой.

 

Отсутствие обработки ошибочных ситуаций

Каждая ошибочная ситуация, если она может повлиять на логику работы программы, должна быть обработана. Однако многие программисты об этом забывают:

pic6

Мало того, что в данном примере может возникнуть дамп в случае возникновения исключения, так еще и их обработка опущена, при этом таблица t_day является критичной относительно дальнейшего поведения программы.

pic7

Если exceptions не важны для логики работы программы, нужно хотя бы поставить OTHERS = 0, чтобы не поймать дамп в самый не подходящий момент.

 

Глобальные переменные

Использование глобальных переменных всегда следует ограничивать, однако многие программисты считают: чем больше глобальных переменных, тем лучше. При этом отследить место, где она изменяется или на что влияет то или иное изменение в больших программах становится очень затруднительно.

К данному примеру можно отнести и глобальные переменные на уровне классов – атрибуты, их изменение/считывание по возможности должно быть всегда через SET/GET методы, иначе сложно будет понять, в каком из методов атрибут был изменен. Кроме того, изменение может сопровождаться доп. проверками, реализовывать которые в 10 методах, где изменяется атрибут, является совсем не верным.

 

Комментарии

Сложные блоки кода должны всегда быть прокомментированы. Комментарии должны быть актуальны, устаревшие комментарии часто сбивают при чтении кода. Комментариев не должно быть много.

Статьи на тему:

 

Хардкодинг

Под данным словом понимаются зашитые в код программы значения, которые могут быть изменены в дальнейшем. Если значение не меняется, а так же встречается в коде неоднократно, следует использовать константы. Следующий пример демонстрирует отсутствие знания разработчика о том, что такое символические константы:

pic8

Кроме того, для иконок есть пул типов, с определенными константами. ‘@1U@’ следует заменить на icon_date.

Еще один пример:

pic9

Что если завтра понадобится еще 1 пользователь? А если этот набор используется в 10 других программах? Вы сможете найти эти программы?

В своем коде вместо подобного хардкодинга, лучше использовать чтение данных из TVARV таблицы (тр. STVARV) или кластера ракурсов, настроечной таблицы и т.п. Измененный аналог:

pic10

 

Копирование стандартных программ

До появления Enhancement Framework, можно было встретить программы, скопированные из стандарта с измененной логикой поведения, обусловлено это было невозможностью расширить ПО под нужды заказчика. В настоящее время от такой практики лучше отказаться, т.к. с каждым новым обновлением в скопированной стандартной программе, Вам потребуется вносить аналогичные изменения в свою Z копию.

 

Забытый код

Очень часто в рамках программы встречаются процедуры, которые никогда не используются, но при этом продолжают находиться в коде программы. В процессе рефакторинга от них следует избавляться, чтобы уменьшить общий объем сопровождаемого кода. Пример из SLIN:

pic11

Так же часто можно встретить огромные блоки кода, которые были закомментированы и так и остались «висеть» в программе, что сказывается на читаемости кода. Чтобы при необходимости вернуть код назад, можно воспользоваться контролем версий.

 

Оператор MESSAGE

Оператор MESSAGE позволяет передавать в качестве сообщения строку:

У этой конструкции есть некоторые недостатки:

  • Вы не сможете добавить перевод к тексту, для этого необходимо изменить вызов следующим образом:

  • Если этот текст будет использован в 10 других программах, при необходимости его менять вам потребуется это сделать везде.
  • Отсутствие подробной документации к тексту.
  • Невозможность быстрого поиска мест, где используется этот текст.

Решением этой проблемы является использование сообщений из класса сообщений: тр. SE91:

 

Нагромождение в EXIT’ах

При использовании расширений на базе user-exit-ов может сложиться ситуация, когда код того или иного расширения будет разрастаться при добавлении функционала. Подобная ситуация была рассмотрена в одной из предыдущих статей.

 

Отсутствие документации

Каждая объемная разработка в системе должна быть так или иначе документирована, т.к. в некоторых случаях разработка затрагивает не единственную Z программу, а целый набор системных расширений с адаптацией стандартного функционала и изменение в одном объекте может нарушить логику поведения остальных.

Кроме документации проектной так же должны быть документированы объекты разработки внутри системы SAP, в противном случае можно встретить вот такие ФМ:

pic12

За что отвечают эти таблицы? Для чего они нужны? Входные это параметры или нет, можно понять относительно префиксов, но так ли это? Выяснить ответы на эти вопросы зачастую совсем не просто, а иногда спасает только отладка.

 

Чрезмерная сложность

Множественное использование одних и тех же выражений:

Вместо:

Данный код работает одинаково, однако во втором случае он более читаем и логичен.

Избыточная вложенность:

Вместо:

Множественные условные переходы:

Вместо:

 

Множественное использование инклудов

Инклуды следует использовать только в рамках разделения логики внутри программы: например, часть кода, отвечающего за работу с БД вынесли в INCLUDE ZDB, часть кода для обработки данных в INCLUDE ZCALC. Не стоит выносить в них определения глобальных переменных или констант для того чтобы использовать в нескольких программах. Может возникнуть ситуация, когда в рамках одного такого инклуда будет множество переменных используемых в какой-то конкретной программе, но не используемых в другой. Кроме того, вы можете захотеть изменить тип переменной (значение константы) в одной программе, при этом, не меняя его в другой, что так же приведет к проблемам.

 

В заключение

Соблюдая перечисленные, не хитрые, правила написания ABAP кода и избегая указанных ошибок, ваш код станет проще как в сопровождении, так и модификации, что может облегчить жизнь не только вам, но и вашим коллегам.

Большинство из правил относительно этой статьи, а также некоторые другие, есть в официальной документации к ABAP: http://help.sap.com/abapdocu_740/en/abenabap_pgl.htm и информации с Wiki.

Литература по теме:

 

  • Сергей

    Здравствуйте.
    Очень правильная статья. С описанными в ней ошибками очень часто сталкиваюсь. Небольшое замечание по разделу «Хардкодинг». Вы, наверно, хотели сказать о том, что разработчик не знает о символических константах, а не просто константах?

    • Astrafox

      На самом деле цифровые константы, так называемые «magic numbers», используемые в 10 разных местах программы без какого либо описания, такое же зло как и символьные. Под константами я имел в виду определение константы как таковое, вне зависимости от типа данных , места объявления и языка программирования.

      • Сергей

        Вы меня не поняли. Да, я согласен, что «magic numbers» — это зло, но я имел ввиду не СИМВОЛЬНЫЕ константы, а СИМВОЛИЧЕСКИЕ. Это разница. Символическая константа — это сопоставленный фактической константе идентификатор.
        Например, 12, ‘SUM_’ — это константы, а вот символические константы:
        CONSTANST:
        co_months_per_year TYPE i VALUE 12,
        co_field_prefix TYPE c LENGTH 4 VALUE ‘SUM_’.

        Уже есть семантика.

        • Astrafox

          Да, все верно, имел в виду именно символические 😉

  • Astrafox

    В дополнение к статье можно ещё упомянуть следующий пункт:
    Определение бизнес-логики в теле макроса, что приводит к невозможности отладки, при неочевидном поведении этой самой логики.

  • Calm

    Спасибо, отличная статья! Полностью согласен.

  • stormio

    Здравствуйте, есть вопрос по поводу высказывания в тексте:

    Если exceptions не важны для логики работы программы, нужно хотя бы поставить OTHERS = 0, чтобы не поймать дамп в самый не подходящий момент.

    Если EXCEPTIONS не важны, то эту секцию при вызове ФМа можно опустить, и тогда sy-subrc будет оставаться 0. Поправьте поажлуйста, если не так.

    • Добрый день! Если при вызове ФМ не будет указано исключение которое возникает в ФМ произойдет дамп. Others = 0 позволяет проигнорировать все возможные исключительные ситуации. Речь идёт о классических исключениях (не ООП). http://help.sap.com/abapdocu_750/en/abapcall_function_parameter.htm#!ABAP_ADDITION_5@5@ (If the call of an exception raised by RAISE does not assign a return code, the program terminates with a runtime error.)

      • stormio

        Большое спасибо за ответ и за весь блог! Продолжайте радовать нас новыми постами =)

        • Всегда пожалуйста 🙂