저는 이렇게 해봤어요. 역시 리팩토링은 재밌는듯.

ListUtils.isEmpty(this.getAnswers()) return null;
Answer answer = this.getAnswerMostLiked();
if (answer.likedMoreThan(2)) return answer; 
return null;

테스트는 아래처럼 ...

@Test public void 답변중Like가_가장많은것이_베스트답변이어야한다() {
    Question q = aQuestion().with(anAnswer().liked(0).build())
                            .with(anAnswer().liked(2).build())
                            .build();
    assertEquals(2, q.getBestAnswer().getSumLike());
}

@benghun 오호 이렇게 빠른 리팩토링을 진행하시다니.. 제가 리팩토링해서 개선한 코드와 비슷하네요. 일부 다른 부분이 있기는 하지만 거의 같네요.

그럼 단위 테스트는 어떻게 자동화할 수 있을까요?

소스 코드는 code 태그 사용하심 정상적으로 볼 수 있습니다. 혹시 상단에 <> 표시 클릭하심 더 편하게 입력할 수 있어요.

@자바지기 위의 답변을 수정했습니당ㅎ

public class Answer implements Comparable<Answer>{
   private int sumLike = 0;
   public void setSumLike(int sumLike){
      this.sumLike = sumLike;
   }
   public int getSumLike(){
      return this.sumLike;
   }
   @Override
   public int compareTo(Answer o){
      int t_ = this.sumLike;
      int o_ = o.getSumLike();
      return t_ < o_ ? 1 : (t_ > o_ ? -1 : 0);
   }
}

set 메소드가 없어서 우선 set 메소드만 넣어봤어요 데헷;; 곁다리 질문1. 기본형 타입의 변수를 쓰지 않고 랩퍼클래스를 사용하시는 이유가 무엇인가요 ? 곁다리 질문2. Question 클래스에서 getBestAnswer() 메소드 작성시에 직접적으로 instance 변수를 가리키지 않고 getAnswers() 메소드를 사용하시는 이유가 무엇인가요?

@김문수 나도 두 가지 내용에 대해서 항상 질문을 가지고 어떻게 설명하는 것이 좋을지 막막할 때가 있다. 질문 1은 네가 별도의 질문으로 빼서 다시 질문하면 좋겠다. 아무래도 이와 관련해서는 다양한 의견이 나올 듯 하니까.

질문 2 또한 다양한 의견이 나올 듯한데 굳이 get 메소드를 통해 호출할 이유가 없다면 필드에 직접 접근해도 괜찮다고 생각한다. 단 get 메소드를 통해 무엇인가 추가적인 작업이 발생하는 경우 get 메소드를 통하는 것이 맞을 듯하다. 나도 이 부분에 대해서는 필드로 접근했다가 get 메소드를 접근했다가 일관성이 없다. 아마 질문 2에 대한 부분도 다양한 이야기가 나오지 않을까?

질문 1, 2 모두 특별히 고민하지 않고 구현하는 경우가 많은데 이번 기회에 같이 한번 고민해 보는 기회가 되면 좋겠네.

@김문수 @자바지기 이야기가 다른 곳으로 세는 것 같지만 ㅎㅎ 제 의견을 말씀드리면요.

질문1) 래퍼 클래스 사용여부

일반적인 경우에 primitive type을 사용합니다. 물론 초를 다루는 시스템이 아니면 상관 없겠지만 별도의 객체를 생성하지 않기 때문에 메모리 효율측면에서도 좋겠구요. 값을 비교할 때도 직관적으로 == 로 비교가 가능합니다. 래퍼클래스의 경우 equals를 사용하거나 아니면 위 코드처럼 intValue()로 가져와서 비교를 해야하니 비용이 더 들어가겠죠. 보통 제가 래퍼 클래스를 사용하는 경우는 null 을 반환할 이유가 있을 때나 객체로 필요할 경우에 사용합니다. (제너릭 등)

질문2) 멤버변수의 getter 이용

getter 메소드는 주로 lightweight accessor로 내부로직 없이 해당 멤버변수의 값을 반환해주는 용도로 많이 쓰는데요. 상속관계에서 private 멤버변수에 접근하기 위해서 쓰는 것 같아요. encapsulation이라고 표현해야하나요. 저도 가끔 구분해서 쓰기도 하는데, 보통은 그때그때 상황에 맞춰 쓰는 것 같네요.

앞에 @benghun님의 답변에서 잘 리팩토링했다. 내가 생각하는 방향과 비슷하다. 내가 생각하는 테스트 코드와 리팩토링 코드를 추가해 보면 다음과 같다.

먼저 "답변의 공감 수가 2보다 큰 경우에 베스트 답변으로 인정한다."에 대한 요구사항을 파악하는 부분은 Answer에 위임한다.

package net.slipp.qna;


import static org.hamcrest.CoreMatchers.*;
import static org.junit.Assert.*;


import org.junit.Test;


public class AnswerTest {
	@Test
	public void isBest() {
		Answer answer = createAnswerWithSumLike(2);
		assertThat(answer.isBest(), is(true));
	}


	static Answer createAnswerWithSumLike(final int sumLike) {
		Answer answer = new Answer() {
			@Override
			public Integer getSumLike() {
				return sumLike;
			}
		};
		return answer;
	}
	
	@Test
	public void isNotBest() throws Exception {
		Answer answer = createAnswerWithSumLike(1);
		assertThat(answer.isBest(), is(false));		
	}
}

위와 같이 테스트 코드를 만들고 이에 대한 Production 코드를 다음과 같이 구현한다.

package net.slipp.qna;
 
public class Answer implements Comparable<Answer> {
    private static final int DEFAULT_BEST_SUM_LIKE = 1;
    
	private Integer sumLike = 0;
    
    public Integer getSumLike() {
        return sumLike;
    }
    
	boolean isBest() {
		if (getSumLike() > DEFAULT_BEST_SUM_LIKE) {
			return true;
		}
		return false;
	}
     
    @Override
    public int compareTo(Answer o) {
        int t_ = this.sumLike.intValue();
        int o_ = o.getSumLike().intValue();
        return t_ < o_ ? 1 : (t_ > o_ ? -1 : 0);
    }
}

위와 같이 베스트 답변인지에 대한 유무는 Answer가 담당하면 Question 코드는 다음과 같이 구현할 수 있다.

package net.slipp.qna;
 
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
 
public class Question {
    private List<Answer> answers;
 
    public List<Answer> getAnswers() {
        return answers;
    }
    
    public Answer getBestAnswer() {
        if (isEmptyAnswer()) {
            return null;
        }
        Answer answer = getTopLikeAnswer();
        if ( answer.isBest() ) {
            return answer;
        }
        return null;
    }


	private boolean isEmptyAnswer() {
		return getAnswers() == null || getAnswers().isEmpty();
	}
    
    private Answer getTopLikeAnswer() {
        List<Answer> sortAnswers = new ArrayList<Answer>();
        sortAnswers.addAll(getAnswers());
        Collections.sort(sortAnswers);
        return sortAnswers.get(0);
    }
}

이에 대한 단위 테스트 코드는 다음과 같다.

package net.slipp.qna;


import static org.hamcrest.CoreMatchers.*;
import static org.junit.Assert.*;


import java.util.Arrays;
import java.util.List;


import org.junit.Test;


public class QuestionTest {
    @Test
    public void getBestAnswer() throws Exception {
        Question dut = createQuestion(createAnswerWithSumLike(1), createAnswerWithSumLike(0),
                createAnswerWithSumLike(3));
        Answer bestAnswer = dut.getBestAnswer();
        assertThat(bestAnswer.getSumLike(), is(3));
    }


    @Test
    public void getBestAnswerDontExisted() throws Exception {
    	Question dut = createQuestion(createAnswerWithSumLike(1), createAnswerWithSumLike(0));
        assertThat(dut.getBestAnswer(), is(nullValue()));
    }
    
    @Test
    public void getBestAnswerHasNotAnswer() throws Exception {
    	Question dut = createQuestion();
        assertThat(dut.getBestAnswer(), is(nullValue()));
    }
    
	private Question createQuestion(final Answer... answers) {
		return new Question() {
            @Override
            public List<Answer> getAnswers() {
                return Arrays.asList(answers);
            }
        };
	}


    private Answer createAnswerWithSumLike(int sumLike) {
    	return AnswerTest.createAnswerWithSumLike(sumLike);
    }
}

QuestionTest와 AnswerTest 코드를 보면 객체의 상태 값을 변경하기 위해 get 메소드를 override하고 있는 것을 확인할 수 있다. set 메소드를 추가할 수 있지만 테스트를 위해 상태 값을 변경할 수 있는 mutable한 상태를 만들 수 있기 때문에 좋은 방법은 아니다. 이와 같이 immutable한 상태를 유지하면서 단위 테스트 코드를 추가하려면 메소드 내에서 필드에 직접 접근하기 보다는 get 메소드를 통해 접근할 경우 단위 테스트가 가능한 상태가 될 것이다. 앞에 @김문수가 질문한 get 메소드를 통한 접근에 있어 한 가지 사용 사례가 될 것으로 생각한다.

단위 테스트를 할 때 가장 짜증나는 작업 중의 하나가 테스트를 위한 객체를 초기화하는 작업이다. 이 부분은 이 답변의 createQuestion() 메소드나 createAnswerWithSumLike 메소드와 같이 하나의 메소드를 통해 해결하거나 @benghum님 답변의 예와 같이 구현할 수도 있겠다. 테스트를 위한 객체를 생성하는 부분은 나도 아직까지 이것이 좋다라고 확신을 가지지 못했다.

@benghun 답변 주신 내용 중에 다음 코드에 대한 내부 코드를 구현해서 공유해 주실 수 있을까요?

@Test public void 답변중Like가_가장많은것이_베스트답변이어야한다() {
    Question q = aQuestion().with(anAnswer().liked(0).build())
                            .with(anAnswer().liked(2).build())
                            .build();
    assertEquals(2, q.getBestAnswer().getSumLike());
}

위와 같이 수도 코드만 보여주었는데요. Question을 생성하는 내부 코드 공유해 주실 수 있을까요? 아니면 이와 관련한 참고 문서라도 공유해 주심 감사하겠습니다.

다양한 답변 글 잘읽어보았습니다.

우선 저는 Question과 Answer에 베스트 답변 여부를 판단하는 로직이 없어도 될 것 같다는 생각을 했어요. Board:Question = 1:n 의 관계이고 Question:Answer = 1:n 의 관계일텐데요. 그럼 두 객체들은 각각 n개만큼 생성되는데 그렇다면 베스트답변여부를 판단하는 로직도 해당 객체안에 n개만큼 존재하겠지요. 또 그만큼 비용도 증가할 것이고요.

그래서 유틸성 클래스(LikeUtils)로 extract class 해보았습니다. 코드는 다음과 같습니다.

import java.util.List;


public class Question {
	private List<Answer> answers;


	public List<Answer> getAnswers() {
		return answers;
	}
}```

public class Answer implements Comparable { private Integer sumLike = 0;

public Integer getSumLike() {
    return sumLike;
}

@Override
public int compareTo(Answer o) {
    return o.getSumLike().compareTo(getSumLike());
}

}```

import java.util.ArrayList;
import java.util.Collections;
import java.util.List;


public class LikeUtils {
	
	public static int BEST_LIKE_MIN_VALUE = 2;
	
	public static Answer getBestAnswer(List<Answer> answers) {
		if (answers == null || answers.isEmpty()) {
			return null;
		}


		Answer answer = getSortedTopAnswer(answers);
		if (answer.getSumLike() >= BEST_LIKE_MIN_VALUE) {
			return answer;
		}


		return null;
	}


	private static Answer getSortedTopAnswer(List<Answer> answers) {
		List<Answer> sortedAnswer = new ArrayList<Answer>(answers);
		Collections.sort(sortedAnswer);
		
		return sortedAnswer.get(0);
	}
}```


테스트코드는 다음과 같습니다.

import static org.hamcrest.CoreMatchers.is; import static org.junit.Assert.assertThat;

import java.util.ArrayList; import java.util.Collections; import java.util.List;

import org.junit.Test;

public class AnswerTest {

public static int MOST_LIKE_SUM = 10;

@Test
public void sortedAnswers() {
    List<Answer> sortedAnswers = createAnswersWithBest();
    Collections.sort(sortedAnswers);

    assertThat(sortedAnswers.get(0).getSumLike(), is(MOST_LIKE_SUM));
}


public static List<Answer> createAnswersWithBest() {
    List<Answer> answers = new ArrayList<Answer>();

    for (int i=0; i<=MOST_LIKE_SUM; i++) {
       answers.add(createAnswer(i));
    }


    return answers;
}

public static List<Answer> createAnswersWithoutBest() {
    List<Answer> answers = new ArrayList<Answer>();

    for (int i=0; i<LikeUtils.BEST_LIKE_MIN_VALUE; i++) {
       answers.add(createAnswer(i));
    }


    return answers;
}

public static Answer createAnswer(final Integer sumLike) {
    return new Answer() {
       @Override
       public Integer getSumLike() {
         return sumLike;  
       }
    };
}

}

```package net.slipp.qna;


import static org.hamcrest.CoreMatchers.*;
import static org.junit.Assert.assertThat;


import java.util.ArrayList;
import java.util.List;


import org.junit.Test;


public class LikeUtilsTest {


	@Test
	public void existsBestAnswer() {
		Answer bestAnswer = LikeUtils.getBestAnswer(AnswerTest.createAnswersWithBest());
		assertThat(bestAnswer.getSumLike(), is(AnswerTest.MOST_LIKE_SUM));
	}
	
	@Test
	public void notExistsBestAnswerCauseNoAnswer() {
		Answer answer = LikeUtils.getBestAnswer(null);
		assertThat(answer, is(nullValue()));
		
		Answer emptyAnswer = LikeUtils.getBestAnswer(new ArrayList<Answer>());
		assertThat(emptyAnswer, is(nullValue()));
	}
	
	@Test
	public void notExistsBestAnswerCauseLessThanBestLikeMinValue() {
		List<Answer> notExistsBestAnswers = AnswerTest.createAnswersWithoutBest();
		Answer answer = LikeUtils.getBestAnswer(notExistsBestAnswers);
		assertThat(answer, is(nullValue()));
	}
}

@자바지기 참고자료는 GOOS에요. 이런 것도 참고가 될려나요. ㅎ https://code.google.com/p/make-it-easy/ 테스트 데이터 생성코드를 "하나의 질문을 답변과 함께, 답변과 함께, ... 만들어라." 라고 읽을 수 있어서 저는 좋더라구요.

public class QuestionTest {
    QuestionBuilder aQuestion() { return new QuestionBuilder(); }
    AnswerBuilder anAnswer() { return new AnswerBuilder(); }


    @Test public void Like가_가장많은것이_베스트답변이어야한다() {    
        Question q = aQuestion().with(anAnswer().liked(0).build())                            
                                .with(anAnswer().liked(2).build())                            
                                .build();    
        assertEquals(2, q.getBestAnswer().getSumLike());
    }
}
class QuestionBuilder {
    List<Answer> answers = new ArrayList<Answer>();
    String problem;


    QuestionBuilder with(Answer a) {
        answers.add(a);
    }
    QuestionBuilder with(String problem) {
        this.problem = problem;
    }
    Question build() {
        Question q = new Question(problem);
        q.add(answers);
        return q;
    }
}
class AnswerBuilder {
    int liked = 0;


    AnswerBuilder liked(int liked) { this.liked = liked; }
    Answer build() { 
        Answer a = new Answer();
        a.setSumLike(liked);
        return a;
    }
} 

@자바지기, @lark 말보다는 코드로 표현하고 자신의 기호에 따라 선택하는 것이 속 편한 것 같기는 하지만 제 의견을 말씀드리면 이렇습니다. 베스트 답변의 기준은 이렇게 생각했었습니다. 1. 하나만 존재해야 한다 2. 최소 like 횟수가 2 이상이어야 한다.

따라서 답변의 목록을 가진 객체가 베스트 답변을 선택해야 한다고 생각했죠. 고로 답변 스스로가 베스트여부를 따지는 것은 나의 하반기 평가를 내가 판단하는 것과 유사한 것이고, 유틸에 베스트 답변의 선정 기준이 있는 것은 특정 비즈니스가 범용 클래스를 오염시킨 것 같네요. 비즈니스를 할 것이라면 비즈니스 객체에게 책임을 할당하는 것이 ...

오염이라는 컨셉은 아래를 개념을 참고해주세요. DDD도 개념 잡기엔 도움이 되는 것 같고. http://c2.com/cgi/wiki?AnticorruptionLayer

@benghun 저도 공유해 주신 소스 코드에서 if (answer.likedMoreThan(2)) return answer; 부분을 보고 한참을 고민했습니다. 답변에 대한 베스트 여부를 판단하는 것이 어디에 있을까에 대해서요.

답변 주신 내용을 보니 질문에서 베스트 여부를 판단하는 것이 맞겠다는 생각이 드네요. 즉, Question.isBest(Answer answer)를 두고 answer.likedMoreThan(2)와 같은 형태로 가면 되겠다는 생각이 드네요. 물론 지금과 같이 isBest 없이 바로 answer.likedMoreThan을 호출해도 되겠고요.

Test Data Builder와 관련한 정보는 감사합니다. 저도 이 부분에 대해 좀 더 사용해보고 경험담을 공유해 보도록 할께요. 아무래도 Test Data 만드는 작업이 생각보다 쉽지 않더라고요. 따라서 개인 또는 프로젝트 만의 Test Data 생성 방식이 존재해야 할 듯 합니다. 그렇지 않으면 Test Data 생성에서 중복이 발생하고 유지하기 힘든 경우가 많더라고요.

@benghun 고로 답변 스스로가 베스트여부를 따지는 것은 나의 하반기 평가를 내가 판단하는 것과 유사한 것이고, 유틸에 베스트 답변의 선정 기준이 있는 것은 특정 비즈니스가 범용 클래스를 오염시킨 것 같네요. 예시를 보니 이해하기가 더 쉽네요. 감사합니다^^

그러고보니 Question 객체에서 베스트답변을 가져오는 메소드는 답변들만 인자로 넘겨주면 될테니, static 메소드로 처리하면 되겠군요.ㅎㅎㅎ

@자바지기 님의 의견과 같이 테스트 데이터를 만드는데 고민이었는데 알려주신 make-it-easy의 직관적인 테스트 데이터 생성 방법이 좋아보이네요. 도움 많이 얻고 갑니다~~!

@lark 제가 샘플로 만든 Question.isBest(Answer answer)는 static을 만들자고 제안을 한 것이 아니라 Question에 isBest를 가져도 괜찮겠다는 의견이었습니다. 반드시 static일 필요는 없다고 생각합니다.