Lee Ho Seong #10

leon-808 · 2023-12-20T08:13:31Z

요약

describe, context, it, expect, tobe 적용

Pull Request 체크리스트

TODO

PR 올리기 전 rebase 동기화를 하셨나요?
마지막 줄에 공백 처리를 하셨나요?
커밋 단위를 의미 단위로 나눴나요?
- 예시
  - 코드 가독성을 위해 메서드를 추출하라
  - if-else 문을 if 문으로 분리하라
  - 불필요한 메서드를 인라인화하라
커밋 본문을 작성하셨나요?
- 예시
  - 함수는 한 가지 일을 해야 한다는 원칙에 따라 메서드를 추출합니다.
  - if-else는 컴파일 시 처리가 되어 재컴파일 없이 수정 할 수 없습니다.
    이에 따라 코드가 실행되는 순간에 실행이 결정되는 if 문으로 수정합니다.

메소드 추출을 통해 한 메소드가 한가지 일만 하도록 크기를 줄였음 추상화 수준을 맞춤으로써 가독성 향상

ch01 sum 예제는 풀이하지 않음

describe, context, it, expect, tobe 구조 적용

간단한 if 문의 리팩토링을 수행

테스트를 위하여 원래 void 였던 updateCar() 함수를 임시로 boolean 으로 변경

jihwooon · 2023-12-21T11:09:42Z

src/ch02/trafficLight.test.ts

+interface TrafficLight {
+  isRed(): boolean;
+  isYellow(): boolean;
+  isGreen(): boolean;
+  updateCar(): boolean;
+}


Suggested change

interface TrafficLight {

isRed(): boolean;

isYellow(): boolean;

isGreen(): boolean;

updateCar(): boolean;

}

interface TrafficLight {

updateCar(): boolean;

}

해당 메서드를 제거하면 어떤 결과가 발생할까요? 이 메서드를 제거가 가능한지 대한 여부를 고민 했으면 좋겠습니다.

isRed, isYellow, isGreen 을 주석 처리해서 제거해도 코드 상의 오류가 발생하지 않습니다.
Red, Yellow, Green 클래스 내의 isRed, isYellow, isGreen 은
인터페이스에서 상속 받은 클래스를 구현한게 아니라
개별적으로 클래스를 구현한 꼴이 된 거 같습니다.

인터페이스에서 코드상에 오류가 발생하지 않는다면 상속 받고 있는 클래스에서도 더 이상 사용되지 않아 제거가 가능 하지 않을까요? 99쪽에 필요없는 코드 제거하기 내용을 읽어보시면 좋을 것 같습니다~

jihwooon · 2023-12-21T11:10:55Z

src/ch02/trafficLight.test.ts

+
+const car = new Car();
+const CYCLE = [new Red(), new Green(), new Yellow()];
+const updateCar = (car: TrafficLight) => car.updateCar();


Suggested change

const updateCar = (car: TrafficLight) => car.updateCar();

const updateCar = (car: TrafficLight) => car.updateCar();

매개 변수는 클래스와 함수가 아닌 인터페이스로 매개변수를 사용하고 있습니다. 왜 인터페이스를 사용 했을까요? 이 부분을 찾아보고 공부를 해보면 어떨까요?

인터페이스를 매개변수로 선언하면 해당 인터페이스를 기반으로 구현한
여러 객체를 매개변수에 할당해줄 수 있기 때문? 이라고 생각합니다.

말씀해주신 부분도 맞지만 더 중요한 개념은 다형성이 있습니다. 이 부분은 매우 중요한 개념이라 반드시 공부를 해보시면 좋겠습니다!!

아 terminology 의 중요성을 다시 한번 깨닫고 갑니다.. 감사합니다

jihwooon · 2023-12-21T11:13:08Z

src/ch01/reportPrimes.test.ts

+const reportIfPrime = (i: number) => {
+  console.log(`${i} is prime`);
+}
+


Suggested change

const reportIfPrime = (i: number) => {

console.log(`${i} is prime`);

}

const reportIfPrime = (i: number) => console.log(`${i} is prime`);

화살표 함수를 사용 하셨는데 화살표 함수의 특징은 한 줄로 표현 할 수 있다는 장점을 가지고 있습니다. 현재 함수에서는 그 장점을 활용하지 않아보입니다.

말씀대로 원본 코드를 수정했습니다

jihwooon · 2023-12-21T11:16:05Z

.eslintrc.js

+  plugins: ['@typescript-eslint', 'jest'],
+  extends: ['airbnb', 'plugin:@typescript-eslint/recommended', 'plugin:jest/recommended'],


prettier로 인해서 레이아웃에 변경이 발생 한 거 같습니다. 이 해당 파일은 커밋 올리기 전에 확인하고 커밋에 올라가지 않도록 주의해야합니다.

prettier 를 이 프로젝트 전체에 적용되지 않도록 하였으나 저 부분이 수정된줄은 미쳐 몰랐습니다.
지적 감사합니다만 이미 여러번 push 되어서 git 을 관리해야할지 막막합니다.

jihwooon · 2023-12-21T11:16:42Z

.eslintrc.js

+    'import/extensions': [
+      'error',
+      'ignorePackages',
+      {
+        js: 'never',
+        ts: 'never',
+      },
+    ],


위와 동일하게 레이아웃 변경이 발생했습니다. 커밋 올리기 전에 다시 한번 확인이 필요합니다.

로컬 원본 코드를 마저 수정하였습니다.

if 문 리팩토링시 map[y][x] 에서 메서드 인식 불가 map 생성 함수를 고쳐서 map[y][x] 가 클래스로 받아들여지도록 해야할듯 싶음

jihwooon · 2023-12-21T11:40:56Z

index.ts

+      if ((map[y][x] === RawTile.STONE || map[y][x] === RawTile.FALLING_STONE)
+        && map[y + 1][x] === RawTile.AIR) {
+        map[y + 1][x] = RawTile.FALLING_STONE;
+        map[y][x] = RawTile.AIR;


map[y][x]은 다음 배열을 참조 하고 있습니다.

const map: RawTile[][] = [ [2, 2, 2, 2, 2, 2, 2, 2], [2, 3, 0, 1, 1, 2, 0, 2], [2, 4, 2, 6, 1, 2, 0, 2],

이 배열은 RawTile 열거형을 참조하고 있어서 map으로부터 메서드를 호출 할 수 없습니다. 참조 타입을 변경하시면 됩니다.

그럼 처음 말씀드렸던대로 첫 RawTile 맵을 기반으로
tile class 들을 담고 있는 새 맵을 만들고 그걸 참조하게 하면 될까요?

책에서는 기존 Tile을 인터페이스로 구현하도록 합니다. 때문에 인터페이스로 참조 타입으로 사용해도 됩니다.

아 오류 해결 내용을 뒤에서 다루나 보네요.. 감사합니다 그럼 한번 만들어보겠습니다.

remove 함수 수정 전까지의 작업 완료

-

클래스로의 코드 이관을 통한 if 문, 함수 리팩터링

jihwooon · 2023-12-22T07:28:48Z

src/ch02/trafficLight.test.ts

+// interface TrafficLight {
+//   isRed(): boolean;
+//   isYellow(): boolean;
+//   isGreen(): boolean;
+//   updateCar(): boolean;
+// }


Suggested change

// interface TrafficLight {

// isRed(): boolean;

// isYellow(): boolean;

// isGreen(): boolean;

// updateCar(): boolean;

// }

if(false) {

function doSomeThing() {

interface TrafficLight {

isRed(): boolean;

isYellow(): boolean;

isGreen(): boolean;

updateCar(): boolean;

}

}

}

의미없는 주석은 doSomeThing 함수에 담아서 실행 되지 않게 하는 것이 좋습니다.

seeAlso:

dead-code

습관을 바꾸겠습니다. 해당 코드는 삭제했습니다.

jihwooon · 2023-12-26T03:42:48Z

.gitignore

@@ -1,4 +1,5 @@
 .idea
 .vscode
+.prettierignore


gitingore 파일에 .prettierignore 를 추가하셨네요. 현재 커밋에서는 이 설정이 어떤 의도로 추가 되어 있는지를 알 수가 없습니다. 해당 파일을 따로 커밋을 두고 의도를 설명하는 편이 좋을 것 같습니다.

아 원자적으로 커밋을 나눠야 하나 보네요.
.prettierignore 파일은 이 프로젝트 전반에 prettier 가 작동하지 않도록 하는 파일입니다.
reset 해서 커밋을 나눠보도록 하겠습니다.

해당 파일만 따로 커밋을 잘 나누셨네요~! 이 부분을 어떻게 해결했는지 블로그나 글로 작성하셔도 좋을 것 같습니다~!

jihwooon · 2023-12-26T03:53:59Z

index.ts

+        map[y + 1][x] = new Stone(new Falling());
        map[y][x] = new Air();
      } else if (map[y][x].isBoxy() && map[y + 1][x].isAir()) {
-        map[y + 1][x] = new Box(new Falling);
+        map[y + 1][x] = new Box(new Falling());
        map[y][x] = new Air();
      } else if (map[y][x].isFallingStone()) {
-        map[y][x] = new Stone(new Resting);
+        map[y][x] = new Stone(new Resting());
      } else if (map[y][x].isFallingBox()) {
-        map[y][x] = new Box(new Resting);
+        map[y][x] = new Box(new Resting());


오타 수정를 위한 커밋은 불필요한 커밋을 생성하기 때문에 이전 커밋에서 수정을 하는 편이 좋습니다.

해당 제언까지 수정해서 push 했습니다.

jihwooon · 2023-12-26T14:16:42Z

index.ts

+  isStony(): boolean;
+  isBoxy(): boolean;
+  moveHorizontal(dx: number): void;
+  moveVertical(dx: number): void;


Suggested change

moveVertical(dx: number): void;

moveVertical(dy: number): void;

moveVertical의 인자값은 dx가 아닌 dy로 표기해야 합니다.

dy 로 로컬 코드 수정했습니다. 지적 고맙습니다.
이를 언급하는 커밋은 따로 만들지 않겠습니다.

jihwooon · 2023-12-26T14:26:26Z

index.ts

+  isLeft() { return false; }
+  isUp() { return false; }
+  isDown() { return false; }
+  handle() { moveToTile(playerx + 1, playery); }


Suggested change

handle() { moveToTile(playerx + 1, playery); }

handle() { moveHorizontal(1); }

책에서는 해당 메서드로 구현 되어 있는데 moveToTile로 왜 변경되었는지가 궁금합니다.

제가 이해하고 작성하기로는 moveHorizontal 과 moveVertical 이 인터페이스에서 상속 받아
클래스 안에 구현된 내장 메소드의 형태로 이전되었습니다.
그래서 책을 따라서 코드를 작성하면 input 이라는 Input 인터페이스를 상속 받는 클래스들은
moveHorizontal 과 moveVertical 을 호출할 수 없으며,
이 함수들은 단독으로도 호출될 수 없기 때문에 빨간줄(에러) 가 발생합니다.
moveToTile 이란 메소드는 playerx, playery 라는 좌표 전역 변수를 재설정하고 있습니다.
그래서 저 부분을 바꾸었습니다.

moveToTile 메서드로 두 가지 일을 하는 것으로 보입니다. moveHorizontal와 moveVertical따로 구분한 이유 중 하나 일 것입니다. 상속 받은 클래스에 호출 할 수 없다고 에러가 발생한 이유가 interface에서 상속 받은 클래스에서 메서드 구현이 제대로 되어 있지 않아서 문제가 발생 할 수 있습니다.

생성자와 추가 인터페이스 도입으로 if문 간소화

개인적으로 사용하는 prettier 를 이 프로젝트에서 작동하지 않게 하는 파일입니다.

-

MinimumProcessor, SumProcessor 클래스 분리하여 컴포지션

-

ts 내부에 동일 클래스명이 있어서 Locker 로 변경

-

jihwooon and others added 6 commits December 19, 2023 17:30

Initial commit

d9bee02

update, draw 함수 리팩터링

bd6746e

메소드 추출을 통해 한 메소드가 한가지 일만 하도록 크기를 줄였음 추상화 수준을 맞춤으로써 가독성 향상

ch01 예제 세가지 풀이와 eslint 따옴표 해결

f964ca5

ch01 sum 예제는 풀이하지 않음

sum.test.ts 풀이

d6e7cb1

describe, context, it, expect, tobe 구조 적용

Input 인터페이스, 클래스로의 코드 이관

87b1262

간단한 if 문의 리팩토링을 수행

신호등 기능 interface, class 로의 코드 이관 및 테스트

f268a65

테스트를 위하여 원래 void 였던 updateCar() 함수를 임시로 boolean 으로 변경

jihwooon reviewed Dec 21, 2023

View reviewed changes

타일 Interface, Class 로의 코드 이관 및 에러

78ab05a

if 문 리팩토링시 map[y][x] 에서 메서드 인식 불가 map 생성 함수를 고쳐서 map[y][x] 가 클래스로 받아들여지도록 해야할듯 싶음

leon-808 force-pushed the leehoseong branch from b94aa52 to 78ab05a Compare December 21, 2023 11:35

jihwooon reviewed Dec 21, 2023

View reviewed changes

leon-808 added 4 commits December 21, 2023 21:29

Map 변수, 생성 함수로 에러 해결 및 PR 제언 토대로 수정

21c83cb

remove 함수 수정 전까지의 작업 완료

Remove 메서드 전문화, colorOfTile 클래스로 코드 이관

207897b

-

if 문 추가 이관을 통해 color 를 draw 함수로 변경

ab5ae4c

-

moveHorizontal, Vertical 인라인화

f223e7b

클래스로의 코드 이관을 통한 if 문, 함수 리팩터링

jihwooon reviewed Dec 22, 2023

View reviewed changes

jihwooon reviewed Dec 26, 2023

View reviewed changes

leon-808 changed the title ~~sum test ts 풀이~~ Lee Ho Seong Dec 27, 2023

leon-808 added 3 commits December 27, 2023 14:22

stony, boxy 메서드 추가 및 stone, box 클래스 리팩터링

6733d98

생성자와 추가 인터페이스 도입으로 if문 간소화

.prettierignore 파일 무시

a8e4e99

개인적으로 사용하는 prettier 를 이 프로젝트에서 작동하지 않게 하는 파일입니다.

조건문에서 부수적인 동작 분리

44826c2

-

leon-808 force-pushed the leehoseong branch from 343e877 to 44826c2 Compare December 27, 2023 05:24

leon-808 added 7 commits December 29, 2023 10:40

updateTile 인라인화와 불필요한 코드 정리

755dc54

-

FallStrategy 클래스로 코드 이관 및 메소드 통합

5a9d2f1

-

Array.ts 전략 패턴 적용

6068dff

MinimumProcessor, SumProcessor 클래스 분리하여 컴포지션

Processor 인터페이스를 생성해 유사한 클래스 통합

20ecf29

-

remove 함수를 하나로 통합하기 위한 인터페이스 도입

f50acda

-

Key 와 Lock 클래스의 통합

0983348

ts 내부에 동일 클래스명이 있어서 Locker 로 변경

전략 패턴 적용을 위한 KeyConfiguration 클래스 도입

7474c00

-

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Lee Ho Seong #10

Lee Ho Seong #10

leon-808 commented Dec 20, 2023

jihwooon Dec 21, 2023 •

edited

Loading

leon-808 Dec 21, 2023

jihwooon Dec 21, 2023

jihwooon Dec 21, 2023

leon-808 Dec 21, 2023

jihwooon Dec 21, 2023

leon-808 Dec 21, 2023

jihwooon Dec 21, 2023 •

edited

Loading

leon-808 Dec 21, 2023

jihwooon Dec 21, 2023

leon-808 Dec 21, 2023

jihwooon Dec 21, 2023 •

edited

Loading

leon-808 Dec 21, 2023

jihwooon Dec 21, 2023 •

edited

Loading

leon-808 Dec 21, 2023

jihwooon Dec 21, 2023

leon-808 Dec 21, 2023

jihwooon Dec 22, 2023 •

edited

Loading

leon-808 Dec 27, 2023

jihwooon Dec 26, 2023

leon-808 Dec 27, 2023

jihwooon Dec 27, 2023

jihwooon Dec 26, 2023 •

edited

Loading

leon-808 Dec 27, 2023

jihwooon Dec 26, 2023

leon-808 Dec 27, 2023

jihwooon Dec 26, 2023

leon-808 Dec 27, 2023 •

edited

Loading

jihwooon Dec 27, 2023

	const updateCar = (car: TrafficLight) => car.updateCar();
	const updateCar = (car: TrafficLight) => car.updateCar();

		plugins: ['@typescript-eslint', 'jest'],
		extends: ['airbnb', 'plugin:@typescript-eslint/recommended', 'plugin:jest/recommended'],

	moveVertical(dx: number): void;
	moveVertical(dy: number): void;

	handle() { moveToTile(playerx + 1, playery); }
	handle() { moveHorizontal(1); }

Lee Ho Seong #10

Are you sure you want to change the base?

Lee Ho Seong #10

Conversation

leon-808 commented Dec 20, 2023

요약

Pull Request 체크리스트

TODO

jihwooon Dec 21, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jihwooon Dec 21, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jihwooon Dec 21, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jihwooon Dec 21, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jihwooon Dec 22, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jihwooon Dec 26, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

leon-808 Dec 27, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jihwooon Dec 21, 2023 •

edited

Loading

jihwooon Dec 21, 2023 •

edited

Loading

jihwooon Dec 21, 2023 •

edited

Loading

jihwooon Dec 21, 2023 •

edited

Loading

jihwooon Dec 22, 2023 •

edited

Loading

jihwooon Dec 26, 2023 •

edited

Loading

leon-808 Dec 27, 2023 •

edited

Loading