Комментарии 13
Пока что, для общего понимания, начнем с мануальной настройки Кондиций.

Это какой-то криндж и краш ))

Если написать «для начала пропишем условия вручную», станет немодно и слишком похоже на старые книжки про конечные автоматы :)
Уважаемый, у вас опечатка и в заголовке и в коде — планировщик это Scheduler
Для этого я сделал класс Condition, вот основные его поля:

1. таймер int setedSeconds
2. скипы int setedSkips

Простите, но set, как и put, является неправильным глаголом, и не изменяется.
Более того, явное использование секунд может не пойти вам на пользу. Попробуйте TimeSpan, после чего не нужно будет гадать, сколько секунд в двух/трех/семи с половиной минутах. Учтитывая, что вы используете это поле для таймера, может быть лучше было бы использовать Condition/Invocation/Firing Delay в качестве имени.


То же самое применимо к setSkips -> SkipCount/NumberOfSkips/LevelsToSkip было бы чуть более понятно, IMHO.


В ту же копилку condition.NextSkip(). Сложно сходу понять, что должен делать этот метод. Я бы, например, ожидаю, что он вернет некий Skip/ISkip(pable) из какой-то коллекции и подготовит следующий элемент к возвращению, либо выкинет исключение, если вернуть нечего.


Я честно до сих пор не могу понять, что это должно быть: SkipNext() — пропусти следующее что-то, NextSkip() — получи следующий "пропуск", DecreaseSkipCount() — уменьши счетчик пропускаемых уровней.


Ну и если я правильно понял, и myCondition.IsReady() проверяет что-то типа LevelsToSkip == 0, т.е. операция "дешевая", то лучше его сделать свойством. Не знаю, умеет ли Unity в свойства, но надеюсь.


Еще порадовали античные delegate() {}, давно такого не видел.

почти в топике, есть ссылка на исходник, взгляните пожалуйста, может там будет чуть яснее. Жду вашей критики =)

Смотрите, я это написал не для того чтобы демотивировать или как-то занизить ваш вклад в этот проект. Я пытаюсь донести то, к чему пришел сам, не имея профильного IT-образования — написание легко-читаемого и понятного кода спасает жизни и экономит человеко-часы. Даже если вы пишите в одиночку, вы будете пересматривать этот фрагмент кода через месяц и будете думать "***** кто это все написал что здесь происходит вообще?"


C# язык достаточно выразительный (verbose), ключевые слова не сокращены до минималистичных def/fn/pub/mod и прочее. Можно писать так, что код будет практически читаться как предложение на английском, и сразу же демонстировать читающему, что здесь происходит.


Microsoft довольно интенсивно навязывает конкретный стандарт написания кода, по умолчанию большинство IDE, линтеров и прочих форматтеров преобразуют то, что вы пишите, к этому стандарту (или чему-то подобному). Это уницифирует кодовые базы разных проектов и упрощает взаимодействие между разработчиками. Позволяет избежать неприятных шуток по поводу UPPERCASE метода START() и тому подобного.


Я может быть слепой, но я честно нигде не вижу ссылки на исходный код. В коде страницы я не нашел ни одного слова, содержащего в себе буковосочетанеи git, присутствующее в адресах самых популярных ресурсов хостинга/контроля версий/обмена кодом. Не могли бы вы прояснить?


А вообще, давайте я тогда наброшу на дизайн. Я не претендую на гуру DDD, но такое впечатление, что у вас вывернута логика наизнанку. Вероятно вам нужен какой-то ICondition, который умеет в bool IsMet {get; }, void AdvanceBy(??? timeElapsed, ??? levels passed). И нужен какой-то IAction, в котором IReadOnlyCollection<ICondition> Conditions {get;}, bool AllConditionsAreMet(), void Invoke(), void AdvanceAllConditionsBy(...).
И тогда в ваш планировщик вы закидываете
var timeOutCond = new TmeOutCondition(TimeSpan.FromMinutes(2));
var multiLevelCond = new LevelSkipCondition(5);
var offerToVisitSite = new ShowOfferBannerAction(){
OfferData = GetOfferData(),
Conditions = new ICondition[] {timeOutCond, multiLevelCond}
};
Ну и далее всякие
void InvokeScheduledActions() {
foreach (var action in _actions)
action.AdvanceAllConditionsBy(timeSinceLastUpdate, levelsSinceLastUpdate);
foreach(var readyAction in _actions.Where(x => x.AllConditionsAreMet()))
readyAction.Invoke();
}

Хорошо, вот вам сразу что бросается в глаза:
nameIsExist — если это проверка не на соответствие названию одноименного магазина автозапчастей, то все же nameExists. В той же строчке вы зачем-то создаете List из существующей коллекции, тем самым гоняя лишнюю память туда-сюда и нагружая GC. Dictionary<TKey, TValue>.KeyCollection имплементирует IEnumerable<TKey>, что позволяет вызвать .Any(x => x.Name.ToUpper().Equals(newConditionNameUpp, ...)). Здесь же вы на каждом (!) шаге вызываете newCondition.Name.ToUpper(), что ужасно, закэшируете эту строку перед вызовом. Если по какой-то причине вы боитесь, что ваш словарь будет изменен во время этой операции, то да, придется создавать список руками, либо вызывать .ToList() до Any(), чтобы создать конкретную копию.


Дочитав до следующей строчки я понимаю, что ваш newCondition технически может быть null, не говоря уже о том что newCondition.Name возвращает поле name как есть, которое приходит в ваш конструктор без проверок. У вас тут двойной потенциальный NRE прямо в первой строчке.
Поэтому условие можно было бы переписать как, например,
if (string.IsNullOrWhiteSpace(newCondition?.Name) || ConditionHandlers.Keys.Any(x => x.Name?.ToUpper().Equals(newCondition.Name, StringComparison.OrdinalIgnoreCase) == true))


И далее в таком же стиле. Думаю, другие могут посоветовать более корректные решения.

Только полноправные пользователи могут оставлять комментарии. Войдите, пожалуйста.