Войти
ПрограммированиеФорумОбщее

Тест на рефакторинг (C#) (2 стр)

Advanced: Тема повышенной сложности или важная.

Страницы: 1 2 3 414 Следующая »
#15
0:08, 15 ноя. 2018

tac
> Все равно еще есть дублирование :)
Clamp в свойство StateDefine.Value засунь.

#16
0:12, 15 ноя. 2018

alexey.ch

+ Показать


P.S. ждем еще другие решения - не подсматривайте ... только пишите полностью код - а то потом начнется - а я мол думал не так а вот так :)

#17
0:16, 15 ноя. 2018

tac
> а то потом начнется - а я мол думал не так а вот так :)
Задачу надо четко формулировать. По сути мой ответ в посте #1 логически является решением задачи.

То, что надо код оптимально переписать - не уточнено.

#18
0:17, 15 ноя. 2018

Наилучшее решение уже предложил ShadowTeolog. Поаплодируем ему.

#19
(Правка: 0:19) 0:18, 15 ноя. 2018
alexey.ch
> То, что надо код оптимально переписать - не уточнено.
вот только не надо )) вопрос стоит ровно так, как стоит - как бы вы отрефакторили бы код? когда тебе ведущий программист говорит - вот тебе чужой код, вот тебе +задачка, отрефактори и добавь функциональность ... то он не будет говорить как рефакторить код ..

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

#20
0:22, 15 ноя. 2018
tac
> как бы вы отрефакторили бы код?
Не вижу этого вопроса в посте #0.
Там написано:
> Теперь, появляется новая задача – нужно уметь прибавлять к текущему значению
> некоторую величину, т.е. если
> StateDefine.Value было 30, то мы хотим вызвать некий метод передать ему
> например плюс 10, и ждать что StateDefine.Value станет 40.
>
> Как вы измените код?
То есть задача стоит - нужно уметь прибавлять к текущему значению некоторую величину. Как вы измените код?

Про то, что надо остальной код править - ни слова.

#21
(Правка: 0:25) 0:25, 15 ноя. 2018

alexey.ch

+ Показать
#22
(Правка: 0:30) 0:29, 15 ноя. 2018

tac

+ Показать
#23
(Правка: 0:43) 0:40, 15 ноя. 2018

kipar
> GetState(agent, state).Value+=10;
у меня другое решение, но подумав - это как раз случай, когда можно использовать т.н. flow pattern ... если обеспечить такие функции вызывающему (а вот Value отдать наверх плохо, особенно если это поле, как в коде, а не свойство)

GetState(agent, state).Set(10);
GetState(agent, state).Add(10);

впрочем это уже дело вкуса, т.к. в противном случае будет

SetState(agent, state, 10);
AddState(agent, state, 10);

но реализация будет все равно близкой

#24
(Правка: 0:45) 0:43, 15 ноя. 2018

Как вариант (я не очень помню шарп, так что пусть оно будет псевдокодом):

class Stuff
{
    StateDefine[StateID] states;

    struct Proxy
    {
        StateDefine state;

        public void operator=(float v)
        {
            if (state != null)
                state.value = clamp(v, 0, 100);
        }

        public void operator+(float a)
        {
            if (state != null)
                this = state.value + a;
        }
        // other operators go here
    }

    public Proxy operator[](Agent agent, State state)
    {
        StateDefine sd = states[StateID(agent, state)];
        return Proxy(sd);
    }
}

stuff[agent, state] = 30;
stuff[agent, state] += 10;

#25
(Правка: 0:46) 0:43, 15 ноя. 2018

tac
Тебе ж давно все написали. AgentAddState избыточный класс, его надо снести. А вместо него достаточно мапы, к которой ты будешь обращаться в одной строке. А ограничения при присваивании - это проблема класса, которому ты присваиваешь значение. И не для того, чтоб лишнюю писанину не разводить, а потому что он впоследствии может наследоваться с перекрытием сих ограничений.

> - Как вы измените код?
> - Как вы отрефакторите код? (для меня синонимы)

Ничеси синонимы. Видать, у тебя своя личная терминология. Хотя в теме "Тест на рефакторинг" всем здоровым людям должно быть понятно, что надо делать.

#26
0:44, 15 ноя. 2018

tac
> GetState(agent, state).Set(10);
А если GetState вернет null?

#27
0:50, 15 ноя. 2018

alexey.ch
> А если GetState вернет null?
ну, это уже можно спрятать внутри - например, вернуть не null, а объект StateDefine с State = null, а уже Set/Add проверят на null не сам объект, а его свойство State

#28
(Правка: 0:53) 0:53, 15 ноя. 2018

alexey.ch написал, как оно примерно может выглядеть. Правда, там теперь лишний делегат создаётся, но он может закэшироваться, а может и не закэшироваться. Ну и пофиг. Или нет. Зависит.

Вообще трудно говорить что-то конкретное про фрагмент кода, не зная ни требований, ни общей картины как оно всё работает и зачем оно всё надо. Даже того, сколько в реальном списке будет значений, 15 штук или 15 тысяч.

Смущают несколько вещей:
1. Название класса должно обозначать какую-то сущность, быть существительным. AgentAddState непонятно что обозначает.
2. Параметр argAgent вроде бы обозначает одного «агента», но его тип называется AgentList, будто это коллекция.
3. Само слово arg в названиях параметров лишнее. Хотя, может это стиль такой.
4. Базовый тип у перечислений AgentList и StateType можно было бы сменить с умолчательного int на byte, если вместимости байта хватает для всех значений. Чего зря раздувать коллекцию...
5. Слово state/состояние, имхо, плохо подходит для обозначения чего-то от 0 до 100. Я бы назвал состояния агента не состояниями, а атрибутами или параметрами или ещё как-то. А состояние агента ― по смыслу это совокупность текущих значений всех его атрибутов.
6. Магические числа в коде: 0 и 100. Ноль ― ещё ладно, а 100 неплохо бы заменить на константу MAX_STATE_VALUE или типа того.
7. Если в коллекции не нашлось подходящего элемента и мы об этом умолчали, это нормально?
8. Раз существует некий класс, который хранит значения неких характеристик неких агентов, и у этого класса есть прям публичный метод, который шарится по этим характеристикам и чего-то в них меняет, то наверное способ хранения этих данных (в List<T> или ещё в чём) ― это детали реализации этого класса и никому другому их знать не стоит. Поэтому список стоит сделать приватным, чтобы работа с данными велась только через публичные методы и при желании/надобности реализацию можно было изменить и остальной код ничего бы не заметил.

tac
> Теперь, появляется новая задача – нужно уметь прибавлять к текущему значению
> некоторую величину, т.е. если StateDefine.Value было 30, то мы хотим вызвать некий метод передать ему
> например плюс 10, и ждать что StateDefine.Value станет 40.
> Как вы измените код?

Первым делом поменяю имя метода. Переписал на свой вкус:

+ Показать

#29
0:55, 15 ноя. 2018

DanielSky
> AgentAddState избыточный класс, его надо снести.
полностью согласен :)

проблема в том что здесь нарушен первый принцип ООП (инкапсуляция)

public class StateDefine
{
       public AgentList Agent;
       public StateType Type;
       public float Value;
}

поля, надо заменить на свойства и Clamp применять в сетере/мутаторе, всё остальное через Find решается одной строкой

Страницы: 1 2 3 414 Следующая »
ПрограммированиеФорумОбщее