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

Бочаров Александр #239

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

Conversation

Geratoptus
Copy link

No description provided.

Copy link

@Folleach Folleach left a comment

Choose a reason for hiding this comment

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

Резюмируя

Разделение классов сделано хорошо, мне оно понравилось.

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

Также хотелось бы видеть более прибранный код: где-то улучшить декомпозицию, где-то исправить стилистические мелочи. Об этом я оставил комментарии ниже.
И не забывай обращать внимание на разнообразные предупреждения IDE, они помогают улучшить общую красоту кода ^_^

Comment on lines +10 to +12
<ItemGroup>
<PackageReference Include="System.Drawing.Common" Version="6.0.0" />
</ItemGroup>

Choose a reason for hiding this comment

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

Почему именно 6.0.0? Есть ведь новее: 9.0.0

cs/TagsCloudVisualization/Program.cs Outdated Show resolved Hide resolved
cs/TagsCloudVisualization/images/1000_1_5_TagCloud.jpg Outdated Show resolved Hide resolved
cs/TagsCloudVisualization/Program.cs Outdated Show resolved Hide resolved
Comment on lines 7 to +13
EndProject
Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Samples", "Samples\Samples.csproj", "{B5108E20-2ACF-4ED9-84FE-2A718050FC94}"
EndProject
Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "TagsCloudVisualization", "TagsCloudVisualization\TagsCloudVisualization.csproj", "{01ADC663-2353-458D-9769-790C224690B2}"
EndProject
Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "TagsCloudVisualizationTests", "TagsCloudVisualizationTests\TagsCloudVisualizationTests.csproj", "{EE74CFE7-F089-4D9E-861E-19C4D4854974}"
EndProject

Choose a reason for hiding this comment

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

Ты понимаешь, что написано в этом файле?
Я тоже нет :)

В новых версиях Visual Studio и Rider можно использовать новый формат: slnx
https://blog.ndepend.com/slnx-the-new-net-solution-xml-file-format/

Он более человеко-читаем:

<Solution>
  <Folder Name="/Test/">
    <Project Path="UnitTests\UnitTests.csproj">
      <Configuration Solution="Release|*" Project="*|*|NoBuild" />
    </Project>
  </Folder>
  <Folder Name="/Application/">
    <Project Path="App\App.csproj" />
  </Folder>
</Solution>

Если сделаешь задачу и будет совсем скучно - можешь попробовать перевести твой sln на этот формат. А так, этот комментарий просто для информации, что вот такое существует

@Folleach
Copy link

О, и ещё, такая гранулярность коммитов даёт понять, как разрабатывалась задача, но как правило, такие подробности излишни

Если уже сделал коммит и хочешь что-то добавить, используй amend.

Например коммиты
Теперь проект структурирован по папочкам
Теперь namespace'ы в соответствии с папочками

Можно было схлопнуть в один.

Или вот эти
Добавил README.MD с картинками
README.MD теперь должен нормально отображаться
Теперь README.MD точно должен нормально отображаться

-Добавил заготовку для RandomPoint
-Добавил значения по умолчанию передаваемым параметрам
-RandomPoint теперь возвращает ожидаемые точки
-RandomPoint исправлены значения по умолчанию
-Добавил тест на плотность и форму
-Поменял рандомизацию на более корректную
-Декомпозировал перегруженный тест на пересечение прямоугольнико в раскладке
-Добавил несколько более базовых тестов
-Внедрен EnumeratorExtension
-Теперь может работать с любой реализацией интерфейса IPointsGenerator
-Основной конструктор оформлен, как primary constructor
-Внедрение нового конструктора DefaultVisualizer
-Теперь имя файла не зависит от locale
-Добавил TearDown, в котором при падении теста сохраняется визуальная раскладка, а также информация о прямоугольниках в раскладке
-Для декомпозиции ввел вспомогательные методы
-layoutCenter запоминается при Setup'е
-ShouldGenerateLayoutThatHasHighTightnessAndShapeOfCircularCloud_WhenOptimalParametersAreUsed исправлено нахождение радиуса
-GetLayoutSize исправлено имя Enumerabl'a, с которым работает метод
-LayoutCenter убран за ненадобностью
-CreateRectangleWithCenter убран (теперь в RectangleExtension)
-PutNextRectangle вместо try-catch теперь используется FirstOrDefault + if
-PutNextRectangle сообщение в исключении теперь более информативно и вынесено в константу
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