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

Выделение библиотеки annotations #65

Merged
merged 11 commits into from
Oct 7, 2023

Conversation

nixel2007
Copy link
Member

@nixel2007 nixel2007 commented Aug 27, 2023

@nixel2007 nixel2007 marked this pull request as ready for review September 9, 2023 21:27
&ВремениИнициализации
Функция РазворачивательАннотаций(&Пластилин Поделка) Экспорт
Рефлектор = Новый Рефлектор();
КонтейнерАннотаций = Рефлектор.ПолучитьСвойство(Поделка, "КонтейнерАннотаций");
Copy link
Member Author

Choose a reason for hiding this comment

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

Знаю, что грязновато, но контейнер в публичный апи я пока отдавать не хочу

Copy link
Member

Choose a reason for hiding this comment

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

А завязь всегда снглтоны регает?

Copy link
Member

Choose a reason for hiding this comment

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

А у тебя же в поделке Разворачиватель сразу закешированный лежит, зачем через контейнер обращаться? Или я опять не понял?

Copy link
Member Author

Choose a reason for hiding this comment

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

Синглтон завязи зависит от характера же.

Copy link
Member Author

Choose a reason for hiding this comment

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

Я убрал разворачиватель из полей поделки, там теперь контейнер только, а разворачиватель локально достаётся в одном месте

Copy link
Member

Choose a reason for hiding this comment

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

Типа

&Завязь
&Характер() 

?

Т.е по дефолту синглтон?

Copy link
Member

Choose a reason for hiding this comment

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

Я убрал разворачиватель из полей поделки, там теперь контейнер только, а разворачиватель локально достаётся в одном месте

Может ты конечно подумал что так сделал, но не сделал

image

image

Copy link
Member Author

Choose a reason for hiding this comment

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

Да, по дефолту синглтон, как и жёлудь.

Copy link
Member Author

Choose a reason for hiding this comment

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

Это ж не поле, а локальная переменная)

Copy link
Member

Choose a reason for hiding this comment

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

Это ж не поле, а локальная переменная)

Перепутал, в фабрике поле.)

@@ -111,6 +112,18 @@
Ожидаем.Что(Верховный).ИмеетТип("Булево");
Ожидаем.Что(ВремениИнициализации).ИмеетТип("Булево");

Если ВремениИнициализации Тогда
ТекстСообщения = СтрШаблон(
"К желудю времени инициализации %1 можно прилеплять только детальки.",
Copy link
Member Author

@nixel2007 nixel2007 Sep 9, 2023

Choose a reason for hiding this comment

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

  1. По идее можно разрешить еще и прилепление других желудей времени инициализации, не вижу тут противоречий
  2. Мне не очень нравится доп проверка в конструкторе ДТОхи, кажется, что это должно быть в фабрике желудей. А в фабрике ОпределениеЖелудя создается два раза, что меня тоже уже очень давно напрягает... Помощь с рефакторингом приветствуется!

Copy link
Member

Choose a reason for hiding this comment

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

  1. Плюсую

Copy link
Member

@sfaqer sfaqer Sep 10, 2023

Choose a reason for hiding this comment

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

  1. Йеп
  2. Не понял, а где смотреть что оно два раза создаётся?

Copy link
Member Author

Choose a reason for hiding this comment

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

  1. В фабрике, два раза конструктор вызывается - из жёлудя и из завязи

Copy link
Member

Choose a reason for hiding this comment

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

  1. В фабрике, два раза конструктор вызывается - из жёлудя и из завязи

А, увидел, ага

Copy link
Member

Choose a reason for hiding this comment

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

  1. В фабрике, два раза конструктор вызывается - из жёлудя и из завязи

Но ты имеешь ввиду что просто в двух местах конструктор используется, а не то что он прям два раза вызывается на одном и том же желуде?

Copy link
Member Author

Choose a reason for hiding this comment

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

Да, используется в двух местах. Постоянно надо синхронизировать изменения и тут и там.

Copy link
Member Author

Choose a reason for hiding this comment

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

  1. Предлагаю оставить пока как есть и решить на уровне BFPP

@sonar-openbsl-ru-qa-bot
Copy link

Passed

Analysis Details

3 Issues

  • Bug0 Bugs
  • Vulnerability0 Vulnerabilities
  • Code Smell3 Code Smells

Coverage and Duplications

  • 90 percent coverage94.23% Coverage (94.80% Estimated after merge)
  • 3 percent duplication0.00% Duplicated Code (0.00% Estimated after merge)

View in SonarQube

@sonar-openbsl-ru-qa-bot
Copy link

Passed

Analysis Details

9 Issues

  • Bug0 Bugs
  • Vulnerability0 Vulnerabilities
  • Code Smell9 Code Smells

Coverage and Duplications

  • 90 percent coverage93.75% Coverage (94.60% Estimated after merge)
  • 3 percent duplication0.00% Duplicated Code (0.00% Estimated after merge)

View in SonarQube

@nixel2007 nixel2007 merged commit e3ceeac into master Oct 7, 2023
22 checks passed
@nixel2007 nixel2007 deleted the feature/annotations-lib branch October 26, 2023 06:40
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.

3 participants