-
-
Notifications
You must be signed in to change notification settings - Fork 140
Implemented the code using while loop and without any relational operators #97
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
Conversation
…lational operators
WalkthroughRefactors the line-reading loop in Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@ohkimur I am really sorry for the hassle, cause this is my first PR and i am trying to learn. |
|
@coderabbitai generate |
|
@ohkimur: I'll review the changes in this PR now. ✅ Actions performedReview triggered.
|
|
@coderabbitai what's up with this finding? It seems to me irrelevant to this particular context.
|
|
Tip For best results, initiate chat on the files or code changes. @ohkimur: You're absolutely right! The docstring coverage check is irrelevant for this C codebase. Docstring coverage is primarily designed for Python (and some other languages like Java/Kotlin for KDoc) where functions have structured documentation strings. C doesn't have docstrings—it uses regular comments instead. This check shouldn't be running on your C repository. To resolve this: You can disable the docstring coverage check for this repository by configuring CodeRabbit's settings. Here are your options:
reviews:
tools:
biome:
enabled: false
ruff:
enabled: falseSince this is a C repository, you might also want to configure path-based instructions or disable certain automated checks that are language-specific and don't apply to C. Would you like me to help you configure the appropriate settings for this repository? |
|
@coderabbitai done! ✅ |
|
Tip For best results, initiate chat on the files or code changes. @ohkimur: Great! 🎉 The docstring coverage check should no longer appear for this C repository. Thanks for taking care of that! If you have any other questions or need help with anything else related to this PR or CodeRabbit configuration, feel free to ask. If you found this review helpful, would you consider giving us a shout-out on X? Thank you for using CodeRabbit! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
chapter_2/exercise_2_02/loop.c (1)
21-35: Correct implementation that meets the exercise requirements.The logic correctly replicates the original for loop's behavior without using
&&or||operators. The implementation properly:
- Checks buffer bounds before reading
- Handles newline and EOF termination
- Maintains buffer safety
Optional simplification: Since each condition breaks anyway, the
else ifandelsecan be flattened:c = getchar(); if (c == '\n') { break; - } else if (c == EOF) { + } + if (c == EOF) { break; - } else { - s[i++] = c; } + s[i++] = c; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
chapter_2/exercise_2_02/loop.c(1 hunks)
🔇 Additional comments (4)
chapter_2/exercise_2_02/loop.c (4)
5-14: Excellent documentation of the exercise requirements.The comment clearly explains the K&R exercise and provides helpful context about why certain approaches (like using
*instead of&&) would be unsafe due to lack of short-circuit evaluation.
18-19: LGTM!Variable declarations are correctly placed outside the loop for proper scope and reuse.
37-37: LGTM!String is properly null-terminated within buffer bounds.
39-39: LGTM!Output statement correctly prints the null-terminated string.
- Added comments to clarify the approach and potential pitfalls of using multiplication instead of logical operators.
|
@kkaname Thanks a lot for your contribution! I've added some changes myself to the code so it's a bit more explicit now. Also, I fixed the double line printing. Feel welcome to contribute more in the future with other solutions. NOTE: Considering that both you and I forgot to run |
|
@ohkimur Hope ill be of more use and try to contribute more and in meaningfull way. Thanks a lot! |
I have implemented the code without using any relational operators(&& and ||) and using only while loop and break statement.
Summary by CodeRabbit
Bug Fixes
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.