2016-10-16 2 views
1
자신의 세터를 작성 똑똑하고 더 우아한 방법이었다, 또는 그 문제에 대한 여부를 사용자 입력을 확인하는 코드가

더 우아한 방법

이 그냥 맞는지 궁금

이상하게 보입니다. 나는 초보자, 그래서 어쩌면이이 문제를 접근 단지 표준 방법이 있지만,하지 않을 경우, 나는 '스마트'방법은 모든

public void setMonth(){ 
    boolean run = true; 
    while(run){ 
     int input = scan.nextInt(); 
     if(input > 0 && input < 13){ 
      this.month = input; 
      run = false; 
     } 
    } 
} 
+4

제안 사항 : 설정자에 사용자 입력을하지 마십시오. 입력기를 설정자에게 전달하여 입력의 유효성을 검사하면됩니다. 메소드 밖에서 입력하라는 메시지. – Li357

+2

이 메소드를 세 단계로 나누어야합니다. 1. getInput() 2. validate() 3. setValue(). 현재이 방법은 선호하지 않는 3 가지 책임을 수행합니다. – techtrainer

+2

표시 방법은 Java 컨텍스트에서 일반적으로 "설정자"라고하는 것이 아닙니다. – hyde

답변

0

규칙 : 당신이 있다면 ... 이메일 명령 행 등에서 읽기, 전송, DB, 파일, 네트워크에 액세스하는 것처럼 세터의 모든 입출력 조작을하지 마십시오

1)

반드시, setXXX를 호출하기 전이나 후에, 그것을위한 최상의 서비스 메소드를 작성해야한다.

2) 가능한 한 작은 로직을 세터에 사용하십시오. 범위 검사와 같은 매개 변수 유효성 검사를 수행하는 것이 좋습니다.

3) Setter는 설계된 한 가지만 수행해야합니다 (값 설정). 부작용이 없어야합니다.

4

먼저 당신이 당신의 코드를 단순화 할 수 있습니다 무엇인지 알고 감사하겠습니다 like :

public void setMonth() { 
    int input = 0; 
    do { 
     input = scan.nextInt(); 
    } while(input <= 0 || input >= 13) 

    this.month = input; 
} 

이 외에도이 기능을 두 가지 기능으로 구분할 수 있습니다. 세터는 값을 설정해야합니다.

public void setMonth(int month) { 
    if(month > 0 && month <= 12) { 
     this.month = month; 
    } else { 
     throw new IllegalArgumentException("Month has to be a value between 1 and 12 inclusively. Actual value was :" + month); 
    } 
} 

public void readMonth() { 
    int input = 0; 
    do { 
     input = scan.nextInt(); 
    } while(input <= 0 || input >= 13) 

    setMonth(input); 
} 

ChiefTwoPencils에서 제안한대로 더 세터를 추가 클래스로 추출 할 수 있습니다. 따라서 사용자 입력과 데이터를 훨씬 더 분리합니다.

class MonthDataHolder { 
    private int month; 

     public MonthDataHolder(int month) { 
      setMonth(month); 
    } 

    public void setMonth(int month) { 
     if(isValidMonthValue(month)) { 
      this.month = month; 
     } else { 
      throw new IllegalArgumentException("Month has to be a value between 1 and 12 inclusively. Actual value was :" + month); 
     } 
    } 

     private boolean isValidMonthValue(month) { 
      return month >= 1 || month <= 12; 
     } 
} 

class InputReader { 

     public MonthDataHolder readMonth() { 
     int input = 0; 
     do { 
      input = scan.nextInt(); 
      } while(input <= 0 || input >= 13) 

      return new MonthDataHolder(input); 
     } 
} 
1

세터와 게터는 뮤 테이터 및 접근로 알려져있다.

제 생각에이 메소드들은 오직 객체의 '상태'를 검색하거나 수정하기위한 것입니다. 그것은 비즈니스 로직을 가져서는 안되며 가능한 한 비즈니스 로직을 적게해서는 안됩니다.

기본적으로 저렴한 계산/비즈니스 로직이 있습니다.

세터가 값을 '설정'만하도록하십시오.

public void setMonth(int month){ 
    this.month = month 
} 

방법은 입력에 따라 값을 검색하는 동안

다른 곳 재사용 할 수 있고 내가 제안하는 것이 당신은 유틸리티 클래스를 만들 수 있으며, 그 방법으로 정적이있다.

class DateUtil { 
    public static int retrieveMonthBasedOnInput(){ 
    } 
} 

이 클래스는 사용자 입력에서 세부 정보를 검색 할 수있는 비즈니스 로직을 가져야한다하여 그 빈 인스턴스의 값을 설정하는 동안 사용되어야한다.

+1

당신은'setMonth()'에서 setter를하지 않습니다. 대신 부작용이있는 어떤 방법으로 혼란 스럽습니다. 좋은 연습이 아니므로, 바로 잡으십시오. –

+0

당신은이 클래스를'DateUtil'에 의존하게 만들었습니다. 확실히 한 달을 매개 변수로 전달하고 외부에서 묻는 메시지가 적절한 디자인입니다. –

+0

@KrzysztofCichocki 알았어요! Util 클래스를 갖는 것이 아마도 더 좋은 방법이라고 생각합니다. – Kamal

0

이 기능은 단순해야하며 하나의 작업 만 수행해야합니다. 어쩌면 당신은 다음과 같이 코드를 작성할 수 있습니다 :

public void setMonth(int month) throws IllegalArgumentException { 
    if(month<0 || month>11) throw new IllegalArgumentException("Month can only be set 0~11"); 
    this.month = month; 
} 

위와 같은 기능을 볼 수 있듯이, 한 달에 하나만 설정하면됩니다. 인수가 올바르지 않은 경우 예외를 throw하십시오. 쓰기 "좋은"stter에 대한

관련 문제