Refactor streamReader: Replace goto Statement with Loop in Recv Method #339
Conversation
Codecov Report
@@ Coverage Diff @@
## master #339 +/- ##
==========================================
+ Coverage 93.65% 93.71% +0.05%
==========================================
Files 17 17
Lines 646 652 +6
==========================================
+ Hits 605 611 +6
Misses 30 30
Partials 11 11
|
YukiBobier
force-pushed
the
remove-goto-statement
branch
from
a3f9b2c
to
ed45113
Compare
|
@sashabaranov I've added a test and modified the branch to address the coverage issue. |
|
Thank you for the PR! While we optimize this section, I've got a couple of comments… |
| respErr := stream.errAccumulator.unmarshalError() | ||
| if respErr != nil { | ||
| err = fmt.Errorf("error, %w", respErr.Error) | ||
| for { |
There was a problem hiding this comment.
Due to added level of indentaion I wonder if we should factor out for body in the separate function (or maybe parts of the for body)
There was a problem hiding this comment.
I agree with you. Extracting the big, deeply nested for-loop into a separate method would be a good idea.
I will do so👍
|
I've done this. But hmm... Coverage issues have occurred... |
YukiBobier
force-pushed
the
remove-goto-statement
branch
2 times, most recently
from
0838124
to
079a1f5
Compare
|
@sashabaranov Additionally, I have responded to all the comments that were made. 👍 Could you please review it again? |
|
@YukiBobier duh, we've got conflicts now due to other stuff |
|
@sashabaranov Oops, I will resolve it👌 |
This commit adds tests to improve coverage before refactoring to ensure that the changes do not break anything.
This commit introduces a refactor to improve the clarity of the control flow within the method. The goto statement can sometimes make the code hard to understand and maintain, hence this refactor aims to resolve that.
This commit improves code readability and maintainability by making the Recv method simpler.
YukiBobier
force-pushed
the
remove-goto-statement
branch
from
079a1f5
to
f0a5b75
Compare
|
@sashabaranov Done👍 |
@sashabaranov I'd appreciate your review on these changes.
Description
This pull request introduces a refactor to the Recv method in the streamReader type, which improves code readability and maintainability.
Main Changes
These changes maintain the functionality of the Recv method, while improving the clarity of the control flow within the function. The goto statement can sometimes make the code hard to understand and maintain, hence this refactor aims to resolve that.
Testing
The changes have been tested to ensure that they don't introduce any regressions.