Skip to content

Conversation

@MikePuzanov
Copy link
Owner

No description provided.

Copy link
Collaborator

@yurii-litvinov yurii-litvinov left a comment

Choose a reason for hiding this comment

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

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

threads[i] = new Thread(() => lazyMulti.Get());
}

foreach (var thread in threads)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
foreach (var thread in threads)
foreach (var thread in threads)

{
thread.Start();
}
foreach (var thread in threads)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
foreach (var thread in threads)
foreach (var thread in threads)

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

Choose a reason for hiding this comment

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

Suggested change
public interface ILazy<T>
public interface ILazy<out T>

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

This comment was marked as resolved.

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

Choose a reason for hiding this comment

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

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

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

This comment was marked as resolved.

This comment was marked as resolved.

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

Choose a reason for hiding this comment

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

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

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


Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change

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

Choose a reason for hiding this comment

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

Suggested change
public class LazyFactory<T>
public static class LazyFactory<T>

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

{
thread.Join();
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change

Assert.Throws<NullReferenceException>(() => LazyFactory.CreateSingleLazy<int>(null));
}
}
} No newline at end of file
Copy link
Collaborator

Choose a reason for hiding this comment

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

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

Copy link
Collaborator

Choose a reason for hiding this comment

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

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

Copy link
Collaborator

Choose a reason for hiding this comment

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

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

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

Choose a reason for hiding this comment

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

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

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

Choose a reason for hiding this comment

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

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

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

This comment was marked as resolved.

Copy link
Collaborator

@yurii-litvinov yurii-litvinov left a comment

Choose a reason for hiding this comment

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

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

Assert.Throws<NullReferenceException>(() => LazyFactory.CreateSingleLazy<int>(null));
}
}
} No newline at end of file
Copy link
Collaborator

Choose a reason for hiding this comment

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

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

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

Choose a reason for hiding this comment

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

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


_value = _supplier();
_supplier = null;
_isGenerate = true;
Copy link
Collaborator

Choose a reason for hiding this comment

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

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

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

Choose a reason for hiding this comment

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

_isGenerated?

Copy link
Collaborator

@yurii-litvinov yurii-litvinov left a comment

Choose a reason for hiding this comment

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

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

Assert.Throws<NullReferenceException>(() => LazyFactory.CreateSingleLazy<int>(null));
}
}
} No newline at end of file
Copy link
Collaborator

Choose a reason for hiding this comment

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

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

Copy link
Collaborator

@yurii-litvinov yurii-litvinov left a comment

Choose a reason for hiding this comment

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

Ага, зачтена

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