[holee] javascript-subway-map-precourse #1

hochan222 · 2021-05-25T03:55:38Z

기능 구현 리스트

이번 문제에서는 목록을 먼저 적어가면서 전체 큰 그림을 그리고 하나하나씩 구현하려고 노력했습니다. 또한, 전체 큰 줄기를 보며 어떤 자료구조를 어디에 써야할지, 어떤 디자인 패턴을 사용할지를 먼저 생각한 뒤 코딩을 했습니다. 우선 폴더 구조를 MVC 패턴에 맞게 나눠놓고 각각의 하위 폴더는 페이지별로 폴더를 만들어 기본 구조를 짰습니다. Model은 localStorage 관련 로직들, View에는 처음 화면에 렌더링 되는 로직들, Controller에는 이벤트에 관한 로직들로 나누어서 짰습니다.

localStorage, dataset, select, DOM, 상태관리, MVC, 기능 분리 등등 많은 것을 배웠고 성장 할 수 있는 시간이었습니다.

commit 메세지 또한 기능 단위로 이전보다 잘 나누려고 했습니다. 코딩 컨벤션과 eslint를 통하여 여러 안티 패턴을 보며 좋은 코드는 무엇인지 기준을 세우는 시간이었습니다.

마지막으로, 여러 PR을 코드리뷰하고, 코드리뷰를 받으면서 더 많이 성장할 수 있었던 5일이어서 좋았습니다. 모두 모두 파이팅 입니다 :)

1. 역 관리 눌렀을 때 해당 HTML tag hidden=false

이유: 더해진 역만 렌더하는거기 때문이다

EOL은newline으로 끝나는게 표준이다.

jwon42

set, map, slice 등 배열 조작 메서드들을 유연하게 사용하는 모습에서 많이 배웁니다.
mvc 패턴 적용을 위한 기능별 함수 분리로 인해 유지보수가 편할 것 같고 코드 가독성이 좋았습니다.
확실히 이전 두 과제 코드보다 전반적으로 훨씬 좋네요.
덕분에 많이많이 배웁니다. 수고하셨어요.

jwon42 · 2021-06-01T01:20:06Z

src/view/line-management/index.js

+export const renderLine = () => {
+  const subwayLine = storage.getLocalStorageMap('subway-line');
+  let result = '';
+  for (const [lineName, value] of [...subwayLine]) {


배열에서 for문을 이렇게 사용할 수 있군요. 배워갑니다.

jwon42 · 2021-06-01T01:21:25Z

src/utils.js

+const getElementClass = (className) => {
+  return document.getElementsByClassName(className);
+};
+
+export const elementIds = {
+  stationManagement: getElementId('station-management'),
+  stationManagerButton: getElementId('station-manager-button'),
+  stationNameInput: getElementId('station-name-input'),
+  stationAddButton: getElementId('station-add-button'),
+  stationDeleteButton: getElementClass('station-delete-button'),
+  stationTableTbody: document.querySelector('tbody'),
+  stationTableTbodyTr: document.querySelectorAll(`tbody tr`),
+  lineManagement: getElementId('line-management'),
+  lineManagerButton: getElementId('line-manager-button'),
+  lineNameInput: getElementId('line-name-input'),
+  lineStartStationSelector: getElementId('line-start-station-selector'),
+  lineEndStationSelector: getElementId('line-end-station-selector'),
+  lineAddButton: getElementId('line-add-button'),
+  lineDeleteButton: getElementClass('lineDeleteButton'),
+  lineTableTbody: document.querySelectorAll('tbody')[1],
+  lineTableTbodyTr: document.querySelectorAll(`table[id=line-table] tr`),
+  sectionManagerButton: getElementId('section-manager-button'),
+  sectionManagement: getElementId('section-management'),
+  sectionLineMenuButton: getElementClass('section-line-menu-button'),
+  sectionStationSelector: getElementId('section-station-selector'),
+  sectionOrderInput: getElementId('section-order-input'),
+  sectionAddButton: getElementId('section-add-button'),
+  sectionDeleteButton: getElementClass('section-delete-button'),
+  sectionButtons: getElementId('section-buttons'),
+  sectionRegister: getElementId('section-register'),
+  sectionManageText: getElementId('section-manage-text'),
+  sectionTableTbody: document.getElementsByTagName('tbody')[2],
+  mapPrintManagerButton: getElementId('map-print-manager-button'),
+};


미리 정의해 놓는 게 너무 유용해보입니다.

jwon42 · 2021-06-01T01:27:21Z

src/controller/line-management/delete-line.js

+  for (const item of document.querySelectorAll(
+    `table[id=line-table] tbody tr`,


querySelectorAll 인자 사용 예시 배워갑니다!

jwon42 · 2021-06-01T01:32:33Z

src/controller/line-management/toggle-display.js

+export const toggleDisplay = () => {
+  elementIds.stationManagement.hidden = true;
+  elementIds.lineManagement.hidden = !elementIds.lineManagement.hidden;
+  elementIds.sectionManagement.hidden = true;
+  if (elementIds.mapPrintManagement) {
+    elementIds.mapPrintManagement.hidden = true;
+  }
+  initSubwayRouteMap();
+};


저도 이 코드를 참고해서 같은 구조로 구현했습니다만,
사실상 하드코딩이라 toggle 해야하는 양이 많아졌을 때는 적합하지 않은 방법 같습니다.
일괄적으로 명령어를 전달하는 방법을 고민해야 할 것 같아요. (div class로 toggle해야하는 버튼들을 가둬두고 데이터속성 등의 방법으로 순회하며 요소에 접근하는 등....?? 잘은 모르겠습니다. 신박한 방법을 알게되면 공유하겠습니다.)

맨처음에 작성한뒤 나중에 보았을때 저는 굉장히 안좋은 패턴이라고 결론 내렸습니다. 저에게 만약에 다실 짤 기회가 주어진다면 https://github.com/transcendence42/javascript-subway-map-precourse/blob/jwon/src/view/01-station/template.js 다음과 같이 작성한 후 메뉴 버튼을 클릭할 때마다 한 페이지를 통으로 생성하고 삭제를 해주는 로직으로 다실 짤 것같습니다.

jwon42 · 2021-06-01T01:37:17Z

src/controller/station-management/add-station.js

+  elementIds.stationNameInput.value = '';
+  elementIds.stationNameInput.focus();


만들어 둔 initLineInput 함수를 공용으로 사용하면 좋을 것 같습니다.

원인, 리팩토링 할때 확인안해서...발생.. 꼭... 확인을하자..

hochan222 · 2021-06-01T04:21:44Z

핳하... 수정 완료했습니다. 앞으로는 리팩토링 후 확인 꼭 하겠습니댜

yhshin0

개인적으로는 컨트롤러 밑에 모델이나 뷰가 있는게 맞는 건지 아리송하네요..

그 외에는 구조적으로 잘 짜신것 같습니다. 수고하셨습니다!

yhshin0 · 2021-06-01T05:14:31Z

src/view/line-management/index.js

+  const subwayLine = storage.getLocalStorageMap('subway-line');
+  let result = '';
+  for (const [lineName, value] of [...subwayLine]) {
+    result += addLineTable({ lineName, value });


lineName과 value를 묶는 이유가 궁금합니다.

for of 문법에서 반환되는 요소를 따로 변수 선언 없이 구조분해할당 받기 위해 다음 문법을 사용했습니다. https://developer.mozilla.org/ko/docs/Web/JavaScript/Reference/Operators/Destructuring_assignment

혹시 더 나아질 수 있는 부분이 있는지 궁금합니다.

yhshin0 · 2021-06-01T05:38:49Z

src/controller/section-management/delete-section-station.js

+    .innerHTML.slice(0, -3);
+  const lineStation = e.currentTarget.dataset.lineStation.slice(0, -7);
+  if (!storage.removeSectionStation(lineName, lineStation)) {
+    alert('역은 2개 이하로 삭제할 수 없습니다.');


이 부분도 controller/error-message.js 로 빼면 좋을 것 같습니다.

앗앗 이부분을 빼먹었군요. 진짜 다음 문제부터는 전체적으로 docs 작성 먼저하고 천천히 한번씩 확인해봐야겠어요.

hochan222 · 2021-06-01T05:51:05Z

개인적으로는 컨트롤러 밑에 모델이나 뷰가 있는게 맞는 건지 아리송하네요..

리뷰해주셔서 감사합니다. 컨트롤러로 인해 수행되는 로직이라서 이거를 어떻게 표현할지 저도 고민을 많이했습니다. 처음 제 의도는 model에는 진짜 localStorage에 직접적으로 관련된 로직들만 놓자는 원칙을 정하자는 의도였습니다. 다른 분들의 코드를 다보고 난후 지금의 생각을 말씀드리면 다른 분들이 짜신 코드처럼 해당 mvc 폴더 내부에 로직이 존재하는게 제일 좋은 패턴인것같습니다!

yechoi42

querySelector를 적극적으로 사용하는 게 인상적이었어요 !
로컬스토리지에 subway-line-list 자료는 등록하고 사용하지 않는 것 같은데 맞나요?
좋은 커리큘럼으로 진행해줘서 짧은 시간안에 많은 것을 배워갑니다 👍

yechoi42 · 2021-06-01T06:19:44Z

src/view/station-management/index.js

+const renderStation = () => {
+  const subwayStation = storage.getLocalStorageMap('subway-station');
+  let result = '';
+  for (const station of [...subwayStation.keys()]) {


for of 문 저장해갑니다,,,

for …of 반복문
for of 반복문은 ES6에 추가된 새로운 컬렉션 전용 반복 구문입니다. for of 구문을 사용하기 위해선 컬렉션 객체가 [Symbol.iterator] 속성을 가지고 있어야만 합니다(직접 명시 가능).

var iterable = [10, 20, 30]; for (var value of iterable) { console.log(value); // 10, 20, 30 }

subway-station 는 value없이 key 값만 둔 이유가 있나요?

제가 잘못 저장을 했네요! 저장할때 storage.getLocalStorageMap 이 아닌 storage.getLocalStorageArray로 저장했어야했는데... 원래 의도는 array 입니다..

yechoi42 · 2021-06-01T06:41:58Z

src/controller/station-management/index.js

+  for (const button of elementIds.stationDeleteButton) {
+    addButtonEvent(button, deleteStation);
+  }


혹시 이벤트 위임 대신 각각 달아준 이유가 있나요?

이 당시는 이벤트 위임을 몰랐습니다 하핳.. 이렇게 써보니 이벤트 위임이 얼마나 편한지 느꼈습니다. 하지만, 계산기에서는 이벤트 위임을 썼다는 사실! 저도 많이 배우고 있습니다 :)

yechoi42 · 2021-06-01T06:43:25Z

src/controller/line-management/index.js

+  addButtonEvent(elementIds.lineManagerButton, toggleDisplay);
+  addButtonEvent(elementIds.lineAddButton, addLine);
+  for (const item of document.querySelectorAll(
+    `table[id=line-table] tbody tr button`,


이런 querySelectorAll 표현도 다음에 활용해봐야겠어요 👍

yechoi42 · 2021-06-01T06:46:19Z

src/view/subway-route-map/index.js

+export const renderSubwayRouteMap = () => {
+  renderMap();
+};


renderSubwayRouteMap 사용하는 부분에서 바로 renderMap()을 호출하면 되지 않을까요? 이 파일에선 renderMap()을 export 하고요,

우선 저의 코드 작성의도는 맨처음에 '틀을 짤때 파일 이름과 같은 형식의 함수틀을 생성하고 그 함수 내에는 구역 별로 함수를 나누고 최대한 단순하게 유지하자' 였습니다. 아래 사진과 같이 전체적으로 구조를 통일하고 싶었습니다. 그러다보니 남겨지게 되었는데 이런 경우는 레거시 코드인지 지금도 많이 고민이 됩니다.

yechoi42 · 2021-06-01T06:48:54Z

src/view/subway-route-map/index.js

+  );
+};
+
+const addStationName = (stationNames) => {


여러 역의 이름을 출력하는 함수라서 addStationNames나 addStationList라고 하면 더 직관적일 것 같아요!!

동의합니다! 세세하게 봐주셔서 감사합니다 :)

yechoi42 · 2021-06-01T06:51:30Z

src/controller/utils.js

+export const addButtonEvent = (submitButton, func) => {
+  submitButton.addEventListener('click', (e) => func(e));
+};
+
+export const removeButtonEvent = (submitButton, func) => {
+  submitButton.removeEventListener('click', (e) => func(e));
+};


이건 이전에 유조가 했던 코멘트랑 비슷한데, 저도 제 개인적인 의견으로는 바로 addEventListener, removeEvenvListener를 사용하는 게 더 가독성 좋은 것 같아요!

저한테는 이전이아니고 현재였습니다.. 하핳.. 저도 깊이 공감하여 계산기부터는 바로 직접 구현했습니다.

hochan222

@yechoi42 리뷰 답변드립니다!

로컬스토리지에 subway-line-list 자료는 등록하고 사용하지 않는 것 같은데 맞나요?

해당 자료는 역 삭제시 노선 관리에서 모두 삭제되었는지 판단할 때 시간복잡도를 낮추기위해서 사용했습니다.

const includeLineList = datasetStation => {
  let ret = storage.getLocalStorageArray('subway-line-list');
  return ret === null ? false : ret.includes(datasetStation);
};

저도 많이 배우고있습니다 👍👍

hochan222 · 2021-06-01T15:32:19Z

src/controller/utils.js

+export const addButtonEvent = (submitButton, func) => {
+  submitButton.addEventListener('click', (e) => func(e));
+};
+
+export const removeButtonEvent = (submitButton, func) => {
+  submitButton.removeEventListener('click', (e) => func(e));
+};


저한테는 이전이아니고 현재였습니다.. 하핳.. 저도 깊이 공감하여 계산기부터는 바로 직접 구현했습니다.

hochan222 · 2021-06-01T15:38:37Z

src/view/station-management/index.js

+const renderStation = () => {
+  const subwayStation = storage.getLocalStorageMap('subway-station');
+  let result = '';
+  for (const station of [...subwayStation.keys()]) {


제가 잘못 저장을 했네요! 저장할때 storage.getLocalStorageMap 이 아닌 storage.getLocalStorageArray로 저장했어야했는데... 원래 의도는 array 입니다..

hochan222 · 2021-06-01T15:39:35Z

src/view/subway-route-map/index.js

+  );
+};
+
+const addStationName = (stationNames) => {


동의합니다! 세세하게 봐주셔서 감사합니다 :)

hochan222 · 2021-06-01T15:45:20Z

src/view/subway-route-map/index.js

+export const renderSubwayRouteMap = () => {
+  renderMap();
+};


우선 저의 코드 작성의도는 맨처음에 '틀을 짤때 파일 이름과 같은 형식의 함수틀을 생성하고 그 함수 내에는 구역 별로 함수를 나누고 최대한 단순하게 유지하자' 였습니다. 아래 사진과 같이 전체적으로 구조를 통일하고 싶었습니다. 그러다보니 남겨지게 되었는데 이런 경우는 레거시 코드인지 지금도 많이 고민이 됩니다.

hochan222 added 30 commits May 25, 2021 12:55

[docs] 구현할 기능 목록 추가

47099c5

[docs] id, class tag, code 형식으로 변경

31bcdf8

feat: 기능 구현에 정의한 html 기본 틀 완성하기

830106e

chore: linter, code formatter, gitignore 추가

79f2f58

feat: page frame

5e55efe

feat: add station-management view

42ba961

chore: prettierrc singleQuote 추가

7d45183

feat: add toggleDisplayStationManagement

7ac3e6d

1. 역 관리 눌렀을 때 해당 HTML tag hidden=false

fix: hidden attribute to false for other page

9cd8222

feat: add controlSubwayRouteMap frame

044d5ad

feat: add toggleDisplayLineManagement

8980a62

feat: add toggleSectionManagement page

79904a5

feat: add station-management frame for html

6eb9db6

docs: complete for id tag

5a307a8

feat: addStationLocalStroage

0393c38

docs: complete localStorage에 등록

efc602e

feat: add error-message

5cd878c

feat: add error message by object

ce33ab5

docs: complete station duplicate && station length err

b9650dc

feat: feature add to localstorage

4b91ba1

style: renderStation이름 구체적으로 변경

82beaea

이유: 더해진 역만 렌더하는거기 때문이다

feat: add render station

a2f75b3

feat: divide utils, control, entire

82bf2c3

feat: add data attribute to html tag

8ae7e2f

feat: add data attribute to button

1a86244

fix: station button id to class

f64f518

fix: elementIds button id to class, && feat: getElementClass

3f8abab

feat: add delete button

dd4bc39

feat: add model for remove

8cca7da

style: apply prettier

446335f

hochan222 added 21 commits May 28, 2021 10:43

refactor: seperate toggle-display.js

3961555

refactor: EOL 추가

2158321

EOL은newline으로 끝나는게 표준이다.

refactor: error-message

a54fe7d

refactor: line-management 페이지 기능별로 파일 쪼갬

d0d9fec

refactor: station management 페이지 delete 로직 파일로 분리

328c8a4

refactor, station-management 페이지 기능 파일별로 리펙토링

095dc4b

refactor: line삭제 하는거 remove에서 용어 delete로 통일화

62fe2c6

refactor: export하단으로 정리, controller

1a447b3

refactor: section management 페이지 기능별로 파일 나눔

41c0f1a

refactor: section-management 페이지 export 정리

c4fe9e5

refactor: seperate toggledisplay, subway-route-map

275a728

refactor: rename all of controller toggleDisplay~ to toggleDisplay

503ca20

refactor: view import 순서

84e4029

chore: lint err --fix

f8247d4

chore: lint modify /'to ' in templete string

6d88798

chore: == to ===

114e705

chore: view page, modify '/ to '

3937cc0

chore: view page, modify '/ to '

f7d30e1

chore: eslint err

d1f782a

chore: add no-restricted-syntax for

2ce680c

chore: lint section checkValidValue 파라미터 개수 줄이기

ccf281a

jwon42 approved these changes Jun 1, 2021

View reviewed changes

jwon42 mentioned this pull request Jun 1, 2021

[yechoi] javascript-subway-map-precourse #3

Closed

fix: 처음 초기값에 아무것도 없을때 입력값 오류 해결,

c8e2682

원인, 리팩토링 할때 확인안해서...발생.. 꼭... 확인을하자..

yhshin0 reviewed Jun 1, 2021

View reviewed changes

yechoi42 approved these changes Jun 1, 2021

View reviewed changes

hochan222 commented Jun 1, 2021

View reviewed changes

hochan222 closed this Jun 4, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[holee] javascript-subway-map-precourse #1

[holee] javascript-subway-map-precourse #1

hochan222 commented May 25, 2021 •

edited

Loading

jwon42 left a comment

jwon42 Jun 1, 2021

jwon42 Jun 1, 2021

jwon42 Jun 1, 2021

jwon42 Jun 1, 2021 •

edited

Loading

hochan222 Jun 1, 2021 •

edited

Loading

jwon42 Jun 1, 2021

hochan222 commented Jun 1, 2021

yhshin0 left a comment

yhshin0 Jun 1, 2021

hochan222 Jun 1, 2021

yhshin0 Jun 1, 2021

hochan222 Jun 1, 2021

hochan222 commented Jun 1, 2021 •

edited

Loading

yechoi42 left a comment

yechoi42 Jun 1, 2021

yechoi42 Jun 1, 2021

hochan222 Jun 1, 2021

yechoi42 Jun 1, 2021

hochan222 Jun 1, 2021 •

edited

Loading

yechoi42 Jun 1, 2021

yechoi42 Jun 1, 2021

hochan222 Jun 1, 2021 •

edited

Loading

yechoi42 Jun 1, 2021

hochan222 Jun 1, 2021

yechoi42 Jun 1, 2021

hochan222 Jun 1, 2021 •

edited

Loading

hochan222 left a comment

hochan222 Jun 1, 2021 •

edited

Loading

hochan222 Jun 1, 2021

hochan222 Jun 1, 2021

hochan222 Jun 1, 2021 •

edited

Loading

		for (const item of document.querySelectorAll(
		`table[id=line-table] tbody tr`,

		elementIds.stationNameInput.value = '';
		elementIds.stationNameInput.focus();

[holee] javascript-subway-map-precourse #1

[holee] javascript-subway-map-precourse #1

Conversation

hochan222 commented May 25, 2021 • edited Loading

jwon42 left a 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

jwon42 Jun 1, 2021 • edited Loading

Choose a reason for hiding this comment

hochan222 Jun 1, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hochan222 commented Jun 1, 2021

yhshin0 left a 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

hochan222 commented Jun 1, 2021 • edited Loading

yechoi42 left a 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

hochan222 Jun 1, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hochan222 Jun 1, 2021 • 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

hochan222 Jun 1, 2021 • edited Loading

Choose a reason for hiding this comment

hochan222 left a comment

Choose a reason for hiding this comment

hochan222 Jun 1, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hochan222 Jun 1, 2021 • edited Loading

Choose a reason for hiding this comment

hochan222 commented May 25, 2021 •

edited

Loading

jwon42 Jun 1, 2021 •

edited

Loading

hochan222 Jun 1, 2021 •

edited

Loading

hochan222 commented Jun 1, 2021 •

edited

Loading

hochan222 Jun 1, 2021 •

edited

Loading

hochan222 Jun 1, 2021 •

edited

Loading

hochan222 Jun 1, 2021 •

edited

Loading

hochan222 Jun 1, 2021 •

edited

Loading

hochan222 Jun 1, 2021 •

edited

Loading