-
Notifications
You must be signed in to change notification settings - Fork 0
Lazy added #1
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
base: master
Are you sure you want to change the base?
Lazy added #1
Conversation
| public T get() { | ||
| if (!counted) { | ||
| counted = true; | ||
| result = supplier.get(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Так supplier оказывается в замыкании и ссылка на него хранится даже когда он уже использован по назначению. Правильнее сделать supplier полем и чтобы он не попадал в замыкание
| } | ||
|
|
||
| @Test | ||
| public void multiThreadLazyShortTest() throws InterruptedException{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Пробела не хватает :)
| } | ||
| assertTrue(job.calledOnce()); | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Кажется, нигде не проверяется, что lazy.get вообще возвращает что-то разумное, и что это каждый раз один и тот же объект
| return new Lazy<T>() { | ||
| private Supplier<T> lazySupplier = supplier; | ||
| private T result; | ||
| private boolean counted = false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Теперь тут три поля, а надо два. counted, вообще говоря, не нужен, по состоянию других полей можно понять, вызывали мы supplier или нет
| public T get() { | ||
| if (!counted) { | ||
| counted = true; | ||
| result = lazySupplier.get(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Надо сбросить ссылку на supplier, когда мы уже получили значение и он нам не нужен, чтобы сборщик мусора смог его собрать
| if (value == notInitialized) { | ||
| synchronized (supplier) { | ||
| if (value == notInitialized) { | ||
| value = supplier.get(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Тут тоже, надо аккуратно избавиться от ссылки на supplier.
| if (!isLong) | ||
| return getShort(); | ||
| else | ||
| return getLong(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Кажется, тут лучше тернарный оператор --- это же просто выражение, значение которого зависит от условия
yurii-litvinov
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Да, теперь всё ок, зачтена
| @@ -0,0 +1,751 @@ | |||
| <?xml version="1.0" encoding="UTF-8"?> | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Оо, выложить .idea/workspace.xml --- это что-то вроде самоубийства :) .gitignore сломался?
No description provided.