Conversation
yurii-litvinov
left a comment
yurii-litvinov
left a comment
There was a problem hiding this comment.
Всё несколько сложнее, чем Вы думаете :) Там ещё где-то надо volatile
| threads[i] = new Thread(() => lazyMulti.Get()); | ||
| } | ||
|
|
||
| foreach (var thread in threads) |
There was a problem hiding this comment.
| foreach (var thread in threads) | |
| foreach (var thread in threads) |
| { | ||
| thread.Start(); | ||
| } | ||
| foreach (var thread in threads) |
There was a problem hiding this comment.
| foreach (var thread in threads) | |
| foreach (var thread in threads) |
| @@ -0,0 +1,7 @@ | |||
| namespace Lazyy | |||
| { | |||
| public interface ILazy<T> | |||
There was a problem hiding this comment.
| public interface ILazy<T> | |
| public interface ILazy<out T> |
| { | ||
| public interface ILazy<T> | ||
| { | ||
| public T Get(); |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
| /// </summary> | ||
| public class LazyFactory<T> | ||
| { | ||
| public static LazySingle<T> CreateSingleLazy(Func<T> supplier) |
There was a problem hiding this comment.
И даже тут комментарии нужны
| lock (this._lockObject) | ||
| { | ||
| _isGenerate = true; | ||
| _value = _supplier(); |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
| namespace Lazyy | ||
| { | ||
| /// <summary> | ||
| /// реаилизация однопоточного режима |
There was a problem hiding this comment.
| /// реаилизация однопоточного режима | |
| /// реализация однопоточного режима |
| private Func<T> _supplier; | ||
| private bool _isGenerate = false; | ||
|
|
||
|
|
| /// <summary> | ||
| /// создает обьекты для работы либо в однопоточном, лио многопоточном режиме | ||
| /// </summary> | ||
| public class LazyFactory<T> |
There was a problem hiding this comment.
| public class LazyFactory<T> | |
| public static class LazyFactory<T> |
И параметр-тип ему не нужен, лучше методы генериками сделать. Тогда не надо будет указывать параметр-тип при вызове.
| { | ||
| thread.Join(); | ||
| } | ||
|
|
| Assert.Throws<NullReferenceException>(() => LazyFactory.CreateSingleLazy<int>(null)); | ||
| } | ||
| } | ||
| } No newline at end of file |
There was a problem hiding this comment.
Нигде не проверяется, что если supplier null, происходит что-то разумное, что если supplier возвращает null, то тоже
There was a problem hiding this comment.
Хорошо, а если supplier возвращает null?
| namespace Lazyy | ||
| { | ||
| /// <summary> | ||
| /// создает обьекты для работы либо в однопоточном, лио многопоточном режиме |
There was a problem hiding this comment.
| /// создает обьекты для работы либо в однопоточном, лио многопоточном режиме | |
| /// создает обьекты для работы либо в однопоточном, либо в многопоточном режиме |
| /// <summary> | ||
| /// создает обьект в многопоточном режиме | ||
| /// </summary> | ||
| public LazyMulti(Func<T> supplierNew) |
There was a problem hiding this comment.
Как-то странно параметр называется
| lock (this._lockObject) | ||
| { | ||
| _isGenerate = true; | ||
| _value = _supplier(); |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
yurii-litvinov
left a comment
yurii-litvinov
left a comment
There was a problem hiding this comment.
Не-а :) Почти правильно, но всё равно есть гонка, и у Вас стремительно заканчиваются попытки
| Assert.Throws<NullReferenceException>(() => LazyFactory.CreateSingleLazy<int>(null)); | ||
| } | ||
| } | ||
| } No newline at end of file |
There was a problem hiding this comment.
Хорошо, а если supplier возвращает null?
| namespace Lazyy | ||
| { | ||
| /// <summary> | ||
| /// реализация многопоточного режима |
There was a problem hiding this comment.
Ну это не совсем правдивый комментарий. Это ведь не реализация pthreads или System.Threading :)
|
|
||
| _value = _supplier(); | ||
| _supplier = null; | ||
| _isGenerate = true; |
There was a problem hiding this comment.
И тут наносит удар volatile и модель памяти многоядерных процессоров... _isGenerate = true другие потоки могут увидеть до _value = _supplier();, что приведёт к возврату null и крешу вызывающего.
| public class LazyMulti<T> : ILazy<T> | ||
| { | ||
| private T _value; | ||
| private bool _isGenerate = false; |
yurii-litvinov
left a comment
yurii-litvinov
left a comment
There was a problem hiding this comment.
Окей, осталось только унифицировать тесты (уже не в счёт попыток). Все варианты Lazy реализуют один интерфейс и должны обладать одинаковым поведением на базовых сценариях (на null, на supplier, который возвращает null, что они не запускают вычисление дважды). Значит, можно написать один набор тестов и скармливать ему Lazy через TestCaseData (или, точнее, скармливать тестам функции, которые производят Lazy, можно прямо методы фабики --- ведь нам надо из теста им supplier передать). Тогда получится, что есть общие тесты на все Lazy, и один тест на многопоточную --- что гонок нет. А на однопоточную Lazy специфические тесты вообще не нужны, получается.
| Assert.Throws<NullReferenceException>(() => LazyFactory.CreateSingleLazy<int>(null)); | ||
| } | ||
| } | ||
| } No newline at end of file |
3 participants
No description provided.