Refactor textRows.readRow to improve readability #1123
Conversation
Refactor textRows.readRow to improve readability: * simplified flow: * reduced maximum indent level * return error early * remove 3 'continue' * remove one type cast * remove one obsolete comment about readLengthEncodedString * -6 lines of code
| } | ||
| return err // err != nil |
There was a problem hiding this comment.
isn't err ignored now?
Seems like this should also have a test case.
There was a problem hiding this comment.
Okay, I just saw the comment above.
This is a semantic change, which I disagree with. We should not introduce any silent fallback behavior. The user requested a parsed time and should receive an error, not something unexpected.
There was a problem hiding this comment.
Well, it looks like I misunderstood the original code. And this shows that it must be refactored to make it easier to follow 😉 .
I will fix my code once the test suite is extended to cover that case.
There was a problem hiding this comment.
This still must be changed before I can approve this change.
Co-authored-by: Inada Naoki <[email protected]>
| } | ||
| return err // err != nil |
There was a problem hiding this comment.
This still must be changed before I can approve this change.
Description
Refactor textRows.readRow to improve readability:
Checklist