koust6u
commented
1 year ago Model 패키지에 대한 정리
-Author
2023/05/03 - 김민종-Contents
package by package 분석과 class by class의 수정고려 사항에 대한 정리.0. 공통 고민 사항❗:
1. Lombok을 통한 setter,getter, constructor의 단순화가 과연 유지 보수 측면에서 필요한가?
$\rightarrow$ Lombok의 라인 수 줄여주는 효과가 trade off 될 만큼 명시화가 중요하단 생각은 안됨.
2. Generic 사용하여 범용성을 증가시키는 것이 옳은가?
$\rightarrow$ 이건 같이 고민해 보아야 할 것.
3. MAGIC_NUMBER들을 별도의 class나 annotation등으로 한곳에 모아둘 필요성이 있을까?
$\rightarrow$ 이것도 같이 고민해 보아야 할 것.
1. geometry
Abstract: Cell을 렌더링 하기 위한 좌표 정보와 크기 정보
1.1 LatticePoint (class)
description: 셀의 위치를 정보와 관련 연산을 제공함.
@Getter @Setter @ToString @NoArgsContstructor @AllArgsConstructor @EqualsAndHash
등과 같은 Lombok plugin을 통해 묵시적으로 생성자를 비롯한 데이터 접근 method를 생성함.
@Log4j를 통한 log 기능 제공 but never used. (아래 부턴 lombok 기능 과 Log4j 설명 생략.)
수정 고려 사항)
Mistype:
-
killNagative $\rightarrow$ killNegative: 오타 수정 필요
Duplicate:
-
killNagative & absoluteValue: killNegative의 기능은 동(同) 클래스 내의 absoluteValue를 호출하는 것이 끝임으로 하나 삭제
Ambiguous:
-
absoluteValue $\rightarrow$ makePositive : 직관성을 이해를 위해 동사형으로 operation 명명
-
getNeighbourhood(): 직관성을 위해 enum typed이나 상수 정의를 통해 수정 고려
-
class Name: LatticePoint $\rightarrow$ cellPosition: 직관적인 이름으로 변경 고려
1.2 LatticeDimension (class)
description: cell의 width 와 height에 대한 정보와 연산 제공
수정 고려 사항)
Ambiguous:
- absoluteValue $\rightarrow$ makePositive : 직관성을 이해를 위해 동사형으로 operation 명명.
- 클래스 이름 모호함 ShapeOfCell 과 같은 직관적 명명 필요한가 에대한 고민을 해 보자.
- localVariable lb:모호성 수정 필요.
1.3 LatticeRectangle (class)
description: LatticeDimension 과 LatticePoint의 정보를 통합.
수정 고려 사항)
-
LatticeDimension을 직접 의존하는 방식 보다 strategy pattern 적용하여 결합도 낮춤과 동시에 유연성 확보 고민해 볼 것.
2. Neighbourhood
Abstract: class 각각의 수정이 아닌 다형성을 바탕으로 package 전체의 구조적 변경이 필요해 보인다.
2.1 LatticePointNeighbourhoodType (Enum)
description: 인접 위치에 대한 정의 타입을 열거형으로 만듬.
참고:
- https://en.wikipedia.org/wiki/Von_Neumann_neighborhood
- https://en.wikipedia.org/wiki/Moore_neighborhood
수정 고려 사항)
- 전체적인 구조의 변경이 필요할 것으로 보임 enum type대신 abstract class나 interface를 도입하여
template method pattern 또는 template/callback pattern등을 이용하여 유연성 및 OCP(개방-폐쇄 원칙)을 개선 할 여지가 보임.
2.2 LatticePointNeighbourhoodPosition (Enum)
description: LatticePointNeighbourhoodPosition의 type을 바탕으로 인접 좌표에 type에 맞게 채워나감.
수정 고려 사항) 1번의 LatticePointNeighbourhoodType과 함께 구조적 변경이 필요해 보임.
2.3 LatticePointNeighbourhood (class)
description: LatticePointNeighbourhood class는 Neighbourhood 패키지 전체의 구현체임.
3. Food
3.1 SimulatedEvolutionWorldLattice (class)
description: cell들이 소비할 수 있는 음식들을 난수성으로 생성해주는 역할로 보임.
또 cell들이 음식을 소비하는 것도 관여하는 것으로 보임. 따라서 SRP(단일 책임 원칙)을 지킬 수 있도록
class를 기능 단위로 분리해줄 필요성이 보인다.
수정 고려 사항)
mistype:
- EABLE_GARDEN_OF_EDEN → ENABLE_GARDEN_OF_EDEN 수정
ambiguous:
- FOOD_PER_DAY → FOOD_PER_TIME_STEP 등의 이름으로 모호성 보완 필요성 보임.
Simplify:
- 절대값 처리의 간략화.
if (x < 0) { x *= -1; } if( y < 0) { y *= -1; }수정을 통해 아래와 같이 간략화 전략도 고민해 보자.
x = Math.abs(x); y = Math.abs(y); - while 문의 간략화.
while (f < FOOD_PER_DAY) { f++; int x = random.nextInt(this.dimensions.getX()); int y = random.nextInt(this.dimensions.getY()); if (x < 0) { x *= -1; } if (y < 0) { y *= -1; } worldMapFood[x][y]++; }수정을 통해 아래와 같이 간략화 할 수 있다.
for(int i = 0; i < FOOD_PER_DAY; i++){ int x = Math.abs(random.nextInt(this.dimensions.getX())); int y = Math.abs(random.nextInt(this.dimensions.getY())); worldMapFood[x][y]++; }👉 @TODO: 위와 같은 로직이 코드 곳곳에 존재 함으로 더 좋은 방법이 있을지 고민 해 볼것! 👈
4. Census
4.1 SimulatedEvalutionPoplationCensus (class)
description: 종류별 cell들의 갯수 data와 증가(increament) 로직을 가짐.
switch문 사용으로 추후 유지보수 측면에서의 단점을 가짐 하지만 해당 method가
LifeCycleStatus에 의존함으로 추후 LifeCyCleStatus와 함께 개선 고려 사항을 작성하겠다.
4.2 SimulatedEvalutionPoplationCensusContainer (class)
description: 세대별 cell들의 인구(?) 수치들을 stack에 저장한다.
(이 클래스는 ComputerKurzweilProperties 에 의존하고 멀티스레딩 환경을 고려하여
작성되었기에 건들이기 살짝 불안해서 조금더 분석해보고 업데이트 하겠습니다. ! )
5. Cell
Abstract: Cell들의 LifeCycle 관리 및 정보들을 총괄하여 관리해준다.
5.1 Orientation, LifeCyCleStatus(Enum)
description:
- Orientation: cell의 이동 방향 정보.
- LifeCycleStatus: Cell의 정보 즉 Old, Adult, Hungry, DEAD와 같은 상태 정보
수정 고려 사항)
앞서 언급한 것과 같이 SimulatedEvalutionPoplationCensus는 LifeCycle에 의존적입니다.
또한 Enum LifeCyCle의 값을 받아와 SimulatedEvalutionPoplationCensus에 generation 별로 Cell들의
정보를 갱신 해 나가는 것을 알 수 있습니다. 하지만 SimulatedEvalutionPoplationCensus에서는 Switch문
을 통해 분류 후 증가 하는 방식을 사용하고 있습니다. 따라서 각각의 상태를 abstract class를 통해
다형성을 주고 빼서 template method 통해 증가 연산을 해줄지 다른 더 좋은 방법이 있는지 같이 고민해 봅시다.
왜냐하면 프로젝트 내에는 상당히 많은 Enum타입의 객체들을 사용합니다.
따라서 프로젝트의 코드 일관성을 위해 어떻게 개선 할지에 대한 팀원들 간의 토의가
필요해 보입니다.
👉 @TODO: Enum 타입의 대한 일관성에 대해 토의 해볼 것. 👈
5.2 LifeCycle (class)
description: > cell들의 lifeCycle에 관여하는 전역적인 정보들을 가지며, 상태 변경에도 관여한다.
수정 고려 사항)
- method의 코드들을 아래와 같이 간략화 가능하다 단 오히려 가독성이 떨어질 우려가 있음.
따라서 condition statement를 외부에 사전 정의하여 사용해 보는 방법도 고민해 볼만하다.
ex1) 수정 전
public void eat(int food) { if (fat + food* FAT_PER_FOOD <= MAX_FAT) { fat += food* FAT_PER_FOOD; } else { fat = MAX_FAT; } }
수정 후
fat = (fat + food * FAT_PER_FOOD <= MAX_FAT) ? fat + food * FAT_PER_FOOD : MAX_FAT;
ex2) 수정 전
public boolean move() { age++; if (fat > 0) { fat--; return true; } else { hunger++; return false; } }수정 후
public boolean move() { age++; return fat > 0 ? (fat-- > 0) : (Math.abs(hunger++) < 0 //항상 false가 되는 조건); }
👉 @TODO: 코드 간략화와 가독성의 가중치에 대해 고민해 볼것. 👈
5.3 CellCore (class)
description: 본격적인 cell의 관리 로직으로 보임. 기능들로 세포 분열, dna증가,감소 로직을 가지고 있다.
개선 고려 사항)
- 이 역시도 코드 수많은 분기로 로직이 구성되어 있음 따라서 statement 별로 정의 와 분리, 결합 등을 할 필요가 있어 보인다.
- SRP 원칙 보장을 위해 서비스 로직(?)은 따로 빼줄 필요가 있을까?
Summary ✅
SOLID 관점(+일부의 예시):
1.전체적으로 SRP 원칙이 위배되고 있다.
1. SimulatedEvolutionWorldLattice → food 생성, 그리고, 소비 등의 여러가지 책임을 가짐
2. CellCore → Cell의 방향 생성, 세포분열, 세포 증감 등의 여러 책임을 가짐2.OCP 및 LSP 원칙 보장을 위해 Strategy 패턴 또는 template method pattern 을 통해 보완 필요성 보임
1. LatticeDimension을 통해 여러가지 Lattice의 크기, 모양 변경에도 유연하게 적용 가능하도록 고민해보자.- 상당히 많은 enumeration type 사용 대신 객체간의 결합도를 낮추고 OCP 원칙을 충족 시키기 위해 templateCall-back 등과 같은 변경에 유연한 패턴을 적용 시켜 보자.
dntks1942
Places with smells
Cell
LifeCycle.java
This java file contains hard to maintain expressions to determine the stage of a cell. For example,
Each expression can be a seperate method. For example,
Neighbourhood
LatticePointNeighbourhood.java
We can apply template design to this section because there are lots of repeating codes for each neighbourhood types.
Before
After
LatticePointNeighbourhoodPosition.java
We could get rid of the switch statement if we apply template design to the LatticePointNeighbourhood class. Instead of using seperate method for getting neighbourhood position, we can move the method to the LatticePointNeighbourhood class.
Before
After
LatticePointNeighbourhoodType.java
This class can be deleted
Food
SimulatedEvolutionWorldLattice.java
letFoodGrow() method has repeating logic thus we can split the logic into different methods.