Skip to content

Conversation

@Shankar-Balajee
Copy link

Commits have been made to the cpp file and header files. Pls review. Thanks

@ojasmaheshwari
Copy link
Collaborator

ojasmaheshwari commented Dec 26, 2024

Hey @Shankar-Balajee,
Sorry for the late review.

This works well and completes the requirements.
However, a few additional code changes are required.

  • You should pass parameter msg as a reference instead of passing it by value as the latter can be a performance disadvantage. You can read about passing by reference value here.
  • You should also mark msg as const as without const you cannot pass argument of type const char* to the function. Dropping const would mean that a constant character pointer can be indirectly modified by msg which would violate the whole point of const. If it is still not clear why const is required, you may refer here.
  • As a code style change, I would recommend renaming msg to message. I will add these code style guidelines to the repository soon.
  • Use \n instead of std::endl. You can read about it more here.

Thanks

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.

2 participants