Lazyy by MikePuzanov · Pull Request #2 · MikePuzanov/HomeworkSem3

MikePuzanov · 2021-09-28T17:18:32Z

No description provided.

yurii-litvinov

Всё несколько сложнее, чем Вы думаете :) Там ещё где-то надо volatile

yurii-litvinov · 2021-09-30T15:18:20Z

Lazyy/Lazyy.Test/MultiThreadTest.cs

+                threads[i] = new Thread(() => lazyMulti.Get());
+            }
+
+            foreach (var thread  in threads)


Suggested change

foreach (var thread in threads)

foreach (var thread in threads)

yurii-litvinov · 2021-09-30T15:18:32Z

Lazyy/Lazyy.Test/MultiThreadTest.cs

+            {
+                thread.Start();
+            }
+            foreach (var thread  in threads)


Suggested change

foreach (var thread in threads)

foreach (var thread in threads)

yurii-litvinov · 2021-09-30T15:19:35Z

Lazyy/Lazyy/ILazy.cs

@@ -0,0 +1,7 @@
+namespace Lazyy
+{
+    public interface ILazy<T>


Suggested change

public interface ILazy<T>

public interface ILazy<out T>

Lazyy/Lazyy/ILazy.cs

+{
+    public interface ILazy<T>
+    {
+        public T Get();


yurii-litvinov · 2021-09-30T15:20:43Z

Lazyy/Lazyy/LazyFactory.cs

+    /// </summary>
+    public class LazyFactory<T>
+    {
+        public static LazySingle<T> CreateSingleLazy(Func<T> supplier)


И даже тут комментарии нужны

Lazyy/Lazyy/LazyMulti.cs

+            lock (this._lockObject)
+            {
+                _isGenerate = true;
+                _value = _supplier();


yurii-litvinov · 2021-09-30T15:24:12Z

Lazyy/Lazyy/LazySingle.cs

+namespace Lazyy
+{
+    /// <summary>
+    /// реаилизация однопоточного режима


Suggested change

/// реаилизация однопоточного режима

/// реализация однопоточного режима

yurii-litvinov · 2021-09-30T15:24:26Z

Lazyy/Lazyy/LazySingle.cs

+        private Func<T> _supplier;
+        private bool _isGenerate = false;
+
+


Suggested change

yurii-litvinov · 2021-09-30T15:25:41Z

Lazyy/Lazyy/LazyFactory.cs

+    /// <summary>
+    /// создает обьекты для работы либо в однопоточном, лио многопоточном режиме
+    /// </summary>
+    public class LazyFactory<T>


Suggested change

public class LazyFactory<T>

public static class LazyFactory<T>

И параметр-тип ему не нужен, лучше методы генериками сделать. Тогда не надо будет указывать параметр-тип при вызове.

yurii-litvinov · 2021-09-30T15:26:17Z

Lazyy/Lazyy/Program.cs

+            {
+                thread.Join();
+            }
+


Suggested change

yurii-litvinov · 2021-10-10T10:35:07Z

Lazyy/Lazyy.Test/SingleThreadTest.cs

+            Assert.Throws<NullReferenceException>(() => LazyFactory.CreateSingleLazy<int>(null));
+        }
+    }
+}


Нигде не проверяется, что если supplier null, происходит что-то разумное, что если supplier возвращает null, то тоже

Хорошо, а если supplier возвращает null?

Не поправлено

yurii-litvinov · 2021-10-10T10:35:42Z

Lazyy/Lazyy/LazyFactory.cs

+namespace Lazyy
+{
+    /// <summary>
+    /// создает обьекты для работы либо в однопоточном, лио многопоточном режиме


Suggested change

/// создает обьекты для работы либо в однопоточном, лио многопоточном режиме

/// создает обьекты для работы либо в однопоточном, либо в многопоточном режиме

yurii-litvinov · 2021-10-10T10:36:25Z

Lazyy/Lazyy/LazyMulti.cs

+        /// <summary>
+        /// создает обьект в многопоточном режиме
+        /// </summary>
+        public LazyMulti(Func<T> supplierNew) 


Как-то странно параметр называется

Lazyy/Lazyy/LazyMulti.cs

+            lock (this._lockObject)
+            {
+                _isGenerate = true;
+                _value = _supplier();


yurii-litvinov

Не-а :) Почти правильно, но всё равно есть гонка, и у Вас стремительно заканчиваются попытки

yurii-litvinov · 2021-10-23T16:10:31Z

Lazyy/Lazyy.Test/SingleThreadTest.cs

+            Assert.Throws<NullReferenceException>(() => LazyFactory.CreateSingleLazy<int>(null));
+        }
+    }
+}


Хорошо, а если supplier возвращает null?

yurii-litvinov · 2021-10-23T16:12:17Z

Lazyy/Lazyy/LazyMulti.cs

+namespace Lazyy
+{
+    /// <summary>
+    /// реализация многопоточного режима


Ну это не совсем правдивый комментарий. Это ведь не реализация pthreads или System.Threading :)

yurii-litvinov · 2021-10-23T16:13:57Z

Lazyy/Lazyy/LazyMulti.cs

+
+                _value = _supplier();
+                _supplier = null;
+                _isGenerate = true;


И тут наносит удар volatile и модель памяти многоядерных процессоров... _isGenerate = true другие потоки могут увидеть до _value = _supplier();, что приведёт к возврату null и крешу вызывающего.

yurii-litvinov · 2021-10-23T16:14:13Z

Lazyy/Lazyy/LazyMulti.cs

+    public class LazyMulti<T> : ILazy<T>
+    {
+        private T _value;
+        private bool _isGenerate = false;


_isGenerated?

yurii-litvinov

Окей, осталось только унифицировать тесты (уже не в счёт попыток). Все варианты Lazy реализуют один интерфейс и должны обладать одинаковым поведением на базовых сценариях (на null, на supplier, который возвращает null, что они не запускают вычисление дважды). Значит, можно написать один набор тестов и скармливать ему Lazy через TestCaseData (или, точнее, скармливать тестам функции, которые производят Lazy, можно прямо методы фабики --- ведь нам надо из теста им supplier передать). Тогда получится, что есть общие тесты на все Lazy, и один тест на многопоточную --- что гонок нет. А на однопоточную Lazy специфические тесты вообще не нужны, получается.

yurii-litvinov · 2021-12-01T16:02:48Z

Lazyy/Lazyy.Test/SingleThreadTest.cs

+            Assert.Throws<NullReferenceException>(() => LazyFactory.CreateSingleLazy<int>(null));
+        }
+    }
+}


Не поправлено

yurii-litvinov

Ага, зачтена

lykwer added 3 commits September 28, 2021 01:15

add needs class and interface

c375b32

add some tests

cae4051

add comments

c368a97

yurii-litvinov requested changes Sep 30, 2021

View reviewed changes

fix code

ea0fe95

yurii-litvinov requested changes Oct 10, 2021

View reviewed changes

fix code

583ef8b

yurii-litvinov requested changes Oct 23, 2021

View reviewed changes

lykwer added 2 commits November 22, 2021 13:23

fix

3178cf3

fix

daed898

yurii-litvinov requested changes Dec 1, 2021

View reviewed changes

lykwer added 2 commits December 6, 2021 16:19

fix test

1fe5b98

rename file

a833011

yurii-litvinov approved these changes Dec 18, 2021

View reviewed changes

	foreach (var thread in threads)
	foreach (var thread in threads)

	/// реаилизация однопоточного режима
	/// реализация однопоточного режима

	public class LazyFactory<T>
	public static class LazyFactory<T>

	/// создает обьекты для работы либо в однопоточном, лио многопоточном режиме
	/// создает обьекты для работы либо в однопоточном, либо в многопоточном режиме

Comments

Conversation

MikePuzanov commented Sep 28, 2021

Uh oh!

yurii-litvinov left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment was marked as resolved.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment was marked as resolved.

Uh oh!

yurii-litvinov left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yurii-litvinov left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yurii-litvinov left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants