Skip to content

Conversation

@XJIE6
Copy link
Owner

@XJIE6 XJIE6 commented Feb 13, 2016

No description provided.

@sproshev
Copy link

Не надо коммитить:

  1. папку .idea и *.iml файлы
  2. папку target

Правило такое: если что-то можно сгенерировать на основе исходников или каких-то конфигов, то это что-то не надо коммитить )


public class LazyFactory {

abstract static class MyLazy<T> implements Lazy {

Choose a reason for hiding this comment

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

private ... implements Lazy<T>

Choose a reason for hiding this comment

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

-0.5

@XJIE6
Copy link
Owner Author

XJIE6 commented Feb 14, 2016

@sproshev

  1. Мы просили руководство, чтобы нам провели ликбез по гиту, но так ничего и не добились. Было бы классно, если бы вы рассказали нам про .gitignore
  2. Я исходил из того, что тестов много не бывает. Удобно иметь как простые, так и сложные тесты (которые могут полностью покрывать простые), ведь если упадёт простой, то найти ошибку проще. Тесты с Null проверяют, что внутри не используется сравнения с Null. Тесты с Supplier проверяют, что внутри нет instanceOf Supplier

public class LazyFactory {

abstract private static class MyLazy<T> implements Lazy<T> {
Boolean got = false;

Choose a reason for hiding this comment

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

Boolean -> boolean

ArrayList<Object> answer = new ArrayList<Object>();

LazyFactoryTester3(final Supplier<T> supplier) {
answer.clear();

Choose a reason for hiding this comment

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

См. выше

@sproshev
Copy link

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

@XJIE6
Copy link
Owner Author

XJIE6 commented Mar 21, 2016

@sproshev
исправил не всё, остальное хочу обсудить на паре

@sproshev
Copy link

зачтено, 8.5/10

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