2016-08-31 6 views
0

У меня возникли проблемы с поиском наилучшего способа реорганизации очень большого класса C# и, в частности, как передать общие свойства/значения из большого класса в извлеченные классы и получить эти измененные значения в основном классе.Большой класс с зависимыми классами и общими свойствами

В начале этот класс был длиной 1000 строк и очень процедурный - он включает вызовы методов и выполнение работы в определенной последовательности. По пути вещи сохраняются в базе данных. Во время процесса есть несколько списков элементов, которые обрабатываются и совместно используются в методах. В конце этого процесса есть множество статистических данных, которые представлены пользователю. Эти статистические данные рассчитываются различными методами по мере обработки. Чтобы дать приблизительный контур - процесс включает в себя кучу случайного выбора, и в конце процесса пользователь видит, сколько случайных элементов, сколько недопустимых записей было выбрано, сколько элементов было из этого подписок и т. Д.

Я читаю «Чистый код» дяди Боба, и я стараюсь, чтобы убедиться, что я реорганизую, что каждый класс делает только 1 вещь.

Таким образом, мне удалось извлечь методы и классы, чтобы уменьшить размер файла (теперь он имеет значение до 450 строк), но проблема, с которой я столкнулась сейчас, заключается в том, что эти разбитые классы требуют значений из основного родительского класса которые будут переданы им и обновлены - эти значения будут использоваться и для других методов/методов класса.

Я разрываюсь, какой это чистый подход:

1) Должен ли я создать кучу частных переменных-членов для хранения статистических значений и списков в главном классе, а затем после вызова в основной класс dependnat, возвращают сложный класс результатов, а затем извлекают эти значения и заполняют/обновляют частные переменные-члены? (Много кода котла пластины таким образом)

ИЛИ

2) Что лучше создать DTO или какой-то контейнер класса, который содержит списки и статистические значения и просто передать его на различные методы класса и методы дочернего класса по ссылке, чтобы создать список значений? Другими словами, я просто передаю этот контейнерный класс, и поскольку это объект, другие классы и методы смогут напрямую манипулировать значениями в нем. Затем в конце процесса значения DTO/container/все, что вы хотите вызвать, будут иметь все конечные результаты в нем, и я могу просто извлечь их из класса контейнера (и в этом случае на самом деле нет необходимости извлекать их и заполнять переменные частного члена основного класса.)

Последний способ у меня есть сейчас, но я чувствую, что это запах кода - все это работает, но это просто кажется «хрупким». Я знаю, что большие классы не велики, но по крайней мере все в 1 большой файл действительно кажется ясным, какие свойства я обновление и т.д.

- ОБНОВЛЕНИЕ -

Некоторые подробнее:

К сожалению, я не могу опубликовать какой-либо фактический код, поскольку он является проприетарным, - попробует придумать фиктивный пример и вставить его, если я получу некоторое время. Один из приведенных ниже комментариев касается реорганизации кода в шагах, и это именно то, что я сделал. Цель класса - это, в конечном счете, одно - создать случайный список вещей - поэтому в единственном публичном методе, который вызывается для этого класса, я отредактировал это до 1 уровня абстракции для каждого «шага».Каждый шаг, независимо от того, является ли это методом в одном классе или имеет смысл разбить его на вспомогательный класс для выполнения подэтапов - он по-прежнему требует доступа к спискам, которые создаются во время процесса, и простым переменным счетчика которые отслеживают статистику.

- ОБНОВЛЕНИЕ -

Вот попытка показывая что-то подобное в коде:

public class RandomList(){ 

    public int Id{get; set;} 
    public int Name{get; set;} 
    public int NumOfInvalidItems {get; set;} 
    public int NumOfFirstChunkItems{get; set;} 
    public int NumOfSecondChunkItems{get; set;} 

    public ICollection<RandomListItem> Items{get; set;} 
} 

public class CreateRandomListService(){ 

    private readonly IUnitOfWork _unitOfWork; 
    private readonly ICreateRandomListValidator _createRandomListValidator; 
    private readonly IRandomSubProcessService _randomSubProcessService; 
    private readonly IAnotherSubProcessService _anotherSubProcessService; 

    private RandomList _randomList; 

    public CreateRandomListService(IUnitOfWork unitOfWork, 
           ICreateRandomListValidator  createRandomListValidator, 
           IRandomFirstChunkFactory randomFirstChunkFactory, 
           IRandomSecondChunkFactory randomSecondChunkFactory){ 
    _unitOfWork = unitOfWork; 
    _createRandomListValidator = createRandomListValidator;  

    _randomFirstChunkService = randomFirstChunkFactory.Create(_unitOfWork); 
    _randomSecondChunkService = randomSecondChunkFactory.Create(_unitOfWork); 
} 


public CreateResult CreateRandomList(CreateRandomListValues createValues){ 

    // validate passed in model before proceeding 
    if(_createRandomListValidator.Validate(createValues)) 
     return new CreateResult({HasErrors:true}); 

    InitializeValues(createValues); // fetch settings from db etc and build up 
    ProcessFirstChunk(); 
    ProcessSecondChunk(); 
    SaveWithStatistics(); 
    createResult.Id = _randomList.Id; 
    return createResult; 


} 

private InitializeValues(CreateRandomListValues createValues){ 

    _createValues = createValues; 
    _createValues.ImportantSetting = _unitOfWork.SettingsRepository.GetImportantSetting(); 
    // etc. 
    _randomList = new RandomList(){ 
     // set initial properties etc. some come from the passed in createValues, some from db 
    } 

} 
private void ProcessFirstChunk(){ 
    _randomFirstChunkService.GetRandomFirstChunk(_createValues); 
} 

private void ProcessSecondChunk(){ 
    _randomSecondChunkService.GetRandomSecondChunk(_createValues); 
} 

private void SaveWithStatistics(){ 

    _randomList.Items _createValues.ListOfItems; 
    _randomList.NumOfInvalidItems = _createValues.NumOfInvalidItems; 
    _randomList.NumOfItemsChosen = _createValues.NumOfItemsChosen; 
    _randomList.NumOfFirstChunkItems = _createValues.NumOfFirstChunkItems; 
    _randomList.NumOfSecondChunkItems = _createValues.NumOfSecondChunkItems; 

    _unitOfWork.RandomThingRepository.Add(_randomList); 
    _unitOfWork.Save(); 
} 
} 

public class RandomFirstChunkService(){ 
    private IUnitOfWork _unitOfWork; 

    public RandomFirstChunkService(IUnitOfWork unitOfWork){ 
     _unitOfWork = unitOfWork; 
    } 

public void GetRandomFirstChunk(CreateRandomListValues createValues){ 
    // do processing here - build up list collection and keep track of counts 
    CallMethodThatUpdatesList(creatValues); 

    // how to return this to calling class? currently just updating values in createValues by reference 
    // can also return a complex class here and extract the values back to the main class' member 
    // variables 
} 

private void CallMethodThatUpdatesList(createRandomListValues createValues){ 
    // do work 

} 
} 
+1

Возможно, дядя Боб должен взять валиум. Менее легкомысленно, если вы планируете отправлять внутренности класса на пару новых классов, вы нарушили все неправильно. Объектом рефакторинга является идентификация единиц, которые могут взаимодействовать друг с другом относительно просто.Найдите группы внутренних элементов, которые не должны знать друг о друге и могут торговать простыми результатами; это ваши строительные блоки. Самое главное, если код * работает * сейчас, возможно, вам стоит просто бросить вокруг него #region и пусть это будет. Это называется «редизайн перегородок», и он работает. –

+0

Да, я сделал рефакторинг «redneck», используя тег #region lol - это решение бедного человека, но немного очищает файл. Спасибо за предложения – user1750537

+0

Вы должны разбить его на «шаги», как мы согласились, но каждый «шаг» должен быть построен на общем базовом классе для общей функциональности или класса полезности. Я бы, наверное, пошел по маршруту утилиты (-ов) и имел один для всего материала БД и по одному для каждого «функционального блока» - НО - они должны быть полностью независимыми и не работать друг с другом ... т.е. БД класс должен быть только DB и возвращать что-то общее для вызывающего. – SledgeHammer

ответ

0

Я бы не "передать вещи вокруг". Я бы не разбил его на отдельные классы только потому, что его 1000 линий. Вы в конечном итоге сделаете это намного более беспорядочным и гораздо более головной болью обслуживания.

Вы не разместили свой код (duh), поэтому его трудно критиковать. Если вы действительно это рассмотрите, я подозреваю, что у вас может быть дублированный код, который может быть реорганизован в методы и т. Д.

Если вы уже избавились от дубликата кода, я бы вытащил всю базу данных в слой DAL.

Если вы действительно хотите сделать его меньше (в зависимости от того, какую небольшую информацию вы предоставили), я бы затем реорганизовал его в «шаги» и создавал родительский контейнерный класс типа рабочего процесса.

Снова трудно сказать, не зная кода.

+0

см. Мое Редактировать выше – user1750537

0

Я не знаю, как именно вам удалось реорганизовать класс далеко, но из вашего объяснения это звучит для меня, как «статистика» является концепцией, которая должна стать предметом, что-то вроде:

interface IStatistic<TOutput> 
{ 
    IEnumerable<TOutput> Calculate(IEnumerable<input-type>); 
} 

Если вы хотите, чтобы отобразить некоторую статистику, вы просто использовать соответствующую статистику:

return new MySpecial().Calculate(myData); 

в случае, если статистические объекты не так легко построить, е они просят по некоторым параметрам и так, то вы можете поставить делегат Func, который создает их:

void DoSomething(Func<IStatistic<string>> factory) 
{ 
    string[] inputData = ... 
    foreach (string line in factory().Calculate(inputData)) 
    { 
     // do something... 
    } 
} 

Как вы упоминаете несколько списков, я полагаю, что ввод типа фактически будет несколько типов ввода. Если это так, то это действительно может иметь смысл поставить своего рода DTO, чтобы просто держать списки:

class RawData 
{ 
    public IEnumerable<type1> Data1 { get; } 
    public IEnumerabel<type2> Data2 { get; } 
    ... 
} 

Заметим, однако, что это не DTO «по книге». Во-первых, он неизменен - ​​есть только геттеры. Во-вторых, он предоставляет только последовательности (IEnumerable), а не сырые списки. Обе меры принимаются, чтобы запретить статистические объекты для манипулирования данными.

1

Жестокий ответ заключается в том, что это зависит ... конечно. Трудно выработать ответ, не читая код, но я бы сказал, что как только вы создали новые классы (с одной целью), эти классы и интерфейсы должны определить, какие объекты данных вам нужно пройти, чтобы решить ваши проблемы. И в этом случае странно, если метод возвращает тот же тип, что и передать его, я также считаю, что манипуляция одним объектом через seriers методов является хрупкой. Представьте, что каждый из вас был сервисом REST; то как бы выглядели эти интерфейсы.

Смежные вопросы