Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

REFACTOR: Reformat OperationStatus initialization in OperationImpl #766

Merged

Conversation

Jaewon-pro
Copy link
Collaborator

πŸ”— Related Issue

⌨️ What I did

  • if-else λ¬Έ λ§ˆμ§€λ§‰μ—μ„œ transitionState(OperationState.COMPLETE);λ₯Ό ν•œλ²ˆλ§Œ ν˜ΈμΆœν•˜λ„λ‘ μˆ˜μ •ν•©λ‹ˆλ‹€.

    • CollectionCountOperationImpl
  • CollectionExistOperationImpl 클래슀의 handleLine() λ©”μ„œλ“œμ—μ„œ,
    matchStatus() λ©”μ„œλ“œλ₯Ό ν˜ΈμΆœν•  λ•Œ NOT_FOUND μΈμžκ°€ 두 번 μ‚¬μš©λ˜λŠ” 뢀뢄을 ν•œ 번만 μ‚¬μš©ν•˜λ„λ‘ μˆ˜μ •ν•©λ‹ˆλ‹€.

  • matchStatus() λ©”μ„œλ“œλ₯Ό ν˜ΈμΆœν•  λ•Œ, μΈμžκ°€ λ§Žμ•„ μ—¬λŸ¬ μ€„λ‘œ μž‘μ„±λœ 경우 OperationStatus 객체λ₯Ό μƒμ„±ν•˜λ„λ‘ ν•©λ‹ˆλ‹€.

    AS-IS

    getCallback().receivedStatus(
            matchStatus(line, EXIST, NOT_EXIST, NOT_FOUND, NOT_FOUND,
                    TYPE_MISMATCH, UNREADABLE));

    TO-BE

    OperationStatus status = matchStatus(line, EXIST, NOT_EXIST,
            NOT_FOUND, TYPE_MISMATCH, UNREADABLE);
    getCallback().receivedStatus(status);

@@ -95,7 +95,7 @@ public void handleLine(String line) {
/* ENABLE_MIGRATION end */
try {
// <result value>\r\n
Long.valueOf(line);
Long.parseLong(line);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

μœ„ 라인이 ν•„μš”ν•œ μ΄μœ κ°€ λ¬΄μ—‡μΈκ°€μš”?
MutatorOperationImpl μ—­μ‹œ Long으둜 λ³€ν™˜ν•΄λ†“κ³  값을 μ‚¬μš©ν•˜μ§€ μ•ŠλŠ” 곳이 μžˆλŠ”λ° μ œκ±°ν•΄λ„ λ˜μ§€ μ•Šμ„κΉŒμš”?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CollectionMutateOperationImpl, MutatorOperationImpl의 경우, 응닡값이 <result value>\r\n인 κ²½μš°μΌλ•Œλ₯Ό μ²˜λ¦¬ν•©λ‹ˆλ‹€.

result valueλŠ” 크게 2κ°€μ§€λ‘œ λ‚˜λˆŒ 수 μžˆμŠ΅λ‹ˆλ‹€.

  1. μ •μˆ˜
    • μ΄λŠ” μ„±κ³΅ν•œ 경우둜, mutate μ—°μ‚°(incr, decr)에 μ„±κ³΅ν–ˆλ‹€λŠ” μ˜λ―Έμž…λ‹ˆλ‹€.
  2. NOT_FOUND와 같은 응닡
    • μ΄λŠ” μ‹€νŒ¨ν•œ 경우둜, keyκ°€ μ—†λŠ” λ“± mutate μ—°μ‚°(incr, decr)을 μ²˜λ¦¬ν•˜μ§€ λͺ»ν–ˆλ‹€λŠ” μ˜λ―Έμž…λ‹ˆλ‹€.

응닡이 μ •μˆ˜κ°€ μ•„λ‹Œ μ—λŸ¬ 메세지인 κ²½μš°λ„ μžˆμœΌλ―€λ‘œ, μ •μˆ˜μΈμ§€ ν™•μΈν•˜λŠ” λ‘œμ§μ€ ν•„μš”ν•˜λ‹€κ³  μƒκ°ν•©λ‹ˆλ‹€.


κΈ°μ‘΄ μ½”λ“œμ—μ„œλŠ” long으둜 λ³€ν™˜ν•œ 값을 μ‚¬μš©ν•˜λŠ” λͺ©μ λ³΄λ‹€λŠ”, 응닡이 μ •μˆ˜μΈμ§€ μ•„λ‹Œμ§€λ‘œ κ΅¬λΆ„ν•˜λŠ” λͺ©μ μ΄ 더 큰 것 κ°™μŠ΅λ‹ˆλ‹€.

λ§Œμ•½ try-catch 문을 μ œκ±°ν•œλ‹€λ©΄ μ•„λž˜μ™€ 같은 ν˜•νƒœλ‘œ λ³€κ²½ν•  수 μžˆμŠ΅λ‹ˆλ‹€.

boolean allDigit = line.chars().allMatch(Character::isDigit); // λ˜λŠ” for loop
if (allDigit) {
  getCallback().receivedStatus(new OperationStatus(true, line));
} else {
  OperationStatus status = matchStatus(line, NOT_FOUND, ...);

  getLogger().debug(status);
  getCallback().receivedStatus(status);
}

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

try-catch문을 μ œκ±°ν•œ ν˜•νƒœλ‘œ λ¦¬νŒ©ν† λ§ν•˜λ©΄ 될 것 κ°™μŠ΅λ‹ˆλ‹€.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Long.parseLong()을 μ‚¬μš©ν•œ try-catch λ¬Έ λŒ€μ‹ , 응닡이 μ •μˆ˜μΈμ§€ ν™•μΈν•˜λŠ” if-else둜 μˆ˜μ •ν–ˆμŠ΅λ‹ˆλ‹€.

CollectionMutateOperationImpl, MutatorOperationImpl ν΄λž˜μŠ€μ—μ„œλ§Œ 응닡을 try-catch둜 λΆ„κΈ° μ²˜λ¦¬ν•˜λ˜ κ²ƒμœΌλ‘œ ν™•μΈν–ˆμŠ΅λ‹ˆλ‹€.

@Jaewon-pro Jaewon-pro force-pushed the refactor/works-#159/operation-status branch from bacec82 to 383fdf0 Compare June 20, 2024 08:10

// <result value>\r\n
boolean allDigit = line.chars().allMatch(Character::isDigit);
if (allDigit) {
getCallback().receivedStatus(new OperationStatus(true, line));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

λ‹€λ₯Έ λΆ€λΆ„κ³Ό 일관성 μžˆλ„λ‘ ifλ¬Έ λ°–μ—μ„œ OperationStatusλ₯Ό μ„ μ–Έν•˜κ³  getCallback().receivedStatus(status) ν˜ΈμΆœν•˜λ©΄ 될 것 κ°™μŠ΅λ‹ˆλ‹€.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

μˆ˜μ •ν–ˆμŠ΅λ‹ˆλ‹€.

@Jaewon-pro Jaewon-pro force-pushed the refactor/works-#159/operation-status branch from 383fdf0 to f45bae3 Compare June 20, 2024 08:21
@Jaewon-pro Jaewon-pro force-pushed the refactor/works-#159/operation-status branch from f45bae3 to a82cfc6 Compare June 20, 2024 08:27
@jhpark816 jhpark816 requested a review from uhm0311 June 20, 2024 09:01
@jhpark816
Copy link
Collaborator

@uhm0311 μ΅œμ’… 리뷰 λ°”λžλ‹ˆλ‹€.

@jhpark816 jhpark816 merged commit 715f73a into naver:develop Jun 20, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants