Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Количество параметров в методе, однострочные конструкции, аннотация методов #8

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

Eremkinig
Copy link

Не используйте в методе больше трех основных параметров
Не используйте однострочные конструкции со сложной логикой
Обязательная аннотация методов

Copy link
Owner

@skyksandr skyksandr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Спасибо за PR. Не каждый день такое случается :)
Написал комментарии к предложенным изменениям. Всё обсуждаемо, если что.

@@ -83,6 +82,45 @@ URLСервиса = "";
// ИмяПользователя - Строка
// ПарольПользователя - Строка
```
- Не используйте в методе больше трех основных параметров. Структура дополнительных параметров определяется четвертым параметром
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Спорный момент. Sandi Metz, достаточно известный и авторитетный разработчик в своих правилах https://thoughtbot.com/blog/sandi-metz-rules-for-developers пишет:
Не используйте более 4 аргументов, элементы структуры тоже считаются.
Для 1С можно написать 5, но прятать аргументы в структуру не лучшая рекомендация.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Начиная с 5 параметров глаз уже "начинает теряться" среди такого количества. Почему же прятать аргументы в структуру не лучшая рекомендация?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Дело в том что в среднем по больнице если у метода большое количество аргументов значит у него с большой вероятностью более одной обязанности (single responsibility principle). И это правило как раз помогает лучше структурировать код на методы, которые делают одну вещь.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Убедительно. Тут по всей видимости у каждого разработчика своя величина, +- 1

КонецПроцедуры
```

- Обязательная аннотация методов. Полноценный комментарий к методу, придерживаясь общих правил комментирования
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Стивен Макконел в книге "Совершенный код" пишет - если ваш код требует комментариев - перепишите его, чтобы не требовал. В случае если это недостижимо ... (далее идет целая глава)
Поэтому я бы не упоминал обязательную аннотацию методов.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

В таком случае следует написать "Аннотация не требуется, см. наименование метода"

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

А есть ли ценность в этой аннотации?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Соглашусь, ценности в этом случаем ноль получается

Comment on lines +260 to +261
РезультатЗапроса = Запрос.Выполнить();
ТаблицаРезультатаЗапроса = РезультатЗапроса.Выгрузить();
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Для данного примера я бы не выносил РезультатЗапроса в отдельную переменную.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@Eremkinig
Copy link
Author

По крайней мере я старался, исходя из своего опыта =)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants