2016-08-31 4 views
0

매우 큰 C# 클래스를 리팩터링하는 가장 좋은 방법을 알아 냈습니다. 특히 큰 클래스의 공유 속성/값을 추출 된 클래스로 전달하고 수정 된 값을 사용할 수있게하는 방법이 있습니다. 메인 클래스.종속 클래스 및 공유 속성이있는 큰 클래스

처음에는이 클래스의 길이가 1000 라인이었고 매우 절차 적이었습니다. 메서드를 호출하고 특정 시퀀스로 작업을 수행하는 것이 었습니다. 그 과정에서 모든 것이 데이터베이스에 지속됩니다. 프로세스가 진행되는 동안 메소드에서 작업되고 공유되는 항목의 목록이 여러 개 있습니다. 이 프로세스가 끝나면 사용자에게 표시되는 일련의 통계가 있습니다. 이 통계는 처리가 진행되는 동안 다양한 방법으로 계산됩니다. 대략적인 개요를 제공하려면 프로세스에 무작위 선택이 필요하며 프로세스가 끝나면 무작위 항목 수, 잘못된 레코드 수, 항목 수 등을 확인하십시오.

나는 Bob 삼촌의 "Clean Code"를 읽었으며, 각 클래스가 단 한가지 일만하는지 리펙터로 확인하려고 노력 중이다.

그래서 파일을 더 작게 유지하기 위해 메소드와 클래스를 추출 할 수 있었지만 (지금은 450 줄까지 사용 가능합니다), 지금 당장 가지고있는 문제는 이러한 깨진 클래스가 주 부모 클래스의 값을 필요로한다는 것입니다 이 값은 다른 메소드/클래스 메소드에도 사용됩니다.

이 나는대로 찢어하고있는 가장 깨끗한 방법입니다 :

1) 나는 다음 주 클래스 '로 호출 한 후 통계 값 및 목록 메인 클래스와를 저장하는 개인 멤버 변수의 무리를 만들어야합니다 dependnat 클래스 메소드를 사용하여 복잡한 결과 클래스를 수신 한 다음이 값을 추출하여 private 멤버 변수를 채우거나 업데이트 할 수 있습니까? (보일러 플레이트 코드를 많이 이런 식으로)

또는

2)가 더 나은 DTO 또는 목록 및 통계 값을 보유하고 바로 다양한 클래스 메소드에 전달 컨테이너 클래스의 일종을 만드는 것입니다 및 값을 목록을 구축하기 위해 참조로 하위 클래스 메서드를? 다른 말로하면이 컨테이너 클래스를 그냥 전달하는 것입니다. 객체이기 때문에 다른 클래스와 메소드가 거기에있는 값을 직접 조작 할 수있게 될 것입니다. 그런 다음 프로세스가 끝나면 DTO/container /라고하는 값을 지정하여 최종 결과를 모두 가져오고 컨테이너 클래스에서 추출 할 수 있습니다.이 경우에는 그것들을 추출하고 메인 클래스의 private 멤버 변수를 채 웁니다.)

후자는 제가 지금 가지고있는 방식입니다 만, 이것은 코드 냄새라고 느낍니다. 모든 것이 작동하지만 단지 "약해"보입니다. 나는 큰 클래스가 큰 것은 아니지만 내가 등을 업데이트하고있는 속성으로 하나의 큰 파일에 모든 것을 가진 적어도이 명확하게 보인다 알고

- UPDATE -

일부 추가 정보를 원하시면 :

불행히도 내가 propriatary 인 것처럼 실제 코드를 게시 할 수는 없습니다. 더미 예제를 만들어 내고 시간이 있으면 붙여 넣으려고합니다. 아래 주석 중 하나는 코드를 단계별로 리팩터링하는 것으로 언급했는데 이것이 바로 내가 한 일입니다. 클래스의 목적은 궁극적으로 일의 무작위 목록을 만드는 것입니다. 그래서이 클래스에 대해 호출되는 유일한 공개 메소드에서 각 단계에 대해 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을 던지고 그대로 두어야합니다. 이것은 "Redneck 리팩토링"이라고하며, 작동합니다. –

+0

예 # 영역 태그 lol을 사용하여 "Reddeck 리팩토링"을 수행했습니다. 이는 가난한 사람의 해결책이지만 파일을 다소 정리합니다. 제안에 감사드립니다 – user1750537

+0

두 단계 모두 동의했지만 "단계"는 공통 기능 또는 유틸리티 클래스의 공통 기본 클래스를 기반으로 만들어야합니다. 아마도 유틸리티 클래스 (es) 경로를 가서 모든 DB 항목과 각 "기능 블록"에 대해 하나씩 가지 겠지만 완전히 독립적이어야하고 서로 작동하지 않아야합니다. 즉 DB 클래스는 DB이어야하며 호출자에게 일반적인 것을 반환해야합니다. – SledgeHammer

답변

0

나는 "주위에 물건을"통과하지 않을 :

- - UPDATE 여기

은 코드에서 비슷한 보여주는에서 시도이다. 그 1000 줄이기 때문에 나는 그것을 분리 된 클래스로 분해하지 않을 것이다. 결국 훨씬 더 복잡해지고 유지 보수의 어려움이 생길 것입니다.

코드를 게시하지 않았으므로 비판하기가 어렵습니다. 만약 당신이 정말로 넘어 간다면, 거기에 중복 된 코드가있어서 메소드 등으로 리팩토링 될 수 있다고 생각합니다.

이미 중복 코드를 제거한 적이 있다면, DAL 레이어에 넣을 수 있습니다.

제공하려는 정보를 기반로하여 작게 만들려면 다음 단계로 리팩터링하고 워크 플로 유형 상위 컨테이너 클래스를 만듭니다.

다시 말하지만, 코드를 모른 채 말하기가 어렵습니다.

+0

위의 편집을 참조하십시오. – user1750537

0
나는 지금까지이 클래스를 리팩토링 관리 얼마나 정확히 모르는

,하지만 당신의 설명에서는 "통계"처럼 나에게 소리 는 객체가되어야 개념이다, 뭔가 같은 :

interface IStatistic<TOutput> 
{ 
    IEnumerable<TOutput> Calculate(IEnumerable<input-type>); 
} 
통계 개체를 구성 할 그 쉽지 않은 경우

return new MySpecial().Calculate(myData); 

, EI : 당신은 몇 가지 통계를 표시 할 때

, 당신은 적절한 통계를 사용 그들은 몇 가지 매개 변수를 요청 그래서, 당신은 그 (것)들을 생성하는 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 서비스 였는지 상상해보십시오. 그러면 인터페이스는 어떻게 생겼을까요?