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.

Sign up for GitHub

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

Jump to bottom

Code Review #7

Open
MartinGaoR opened this issue Apr 16, 2016 · 1 comment
Open

Code Review #7

MartinGaoR opened this issue Apr 16, 2016 · 1 comment

Comments

@MartinGaoR
Copy link

@khlee1886 Your Part is ok, but still have magic strings/numbers. Also, comments may be insufficient to describe 'what to do' in your program. Logging is missing in your component.
Possible recommendation: Eliminate magic strings and numbers. Try to implement logging in your part.

@JohannLee The comment in your part is inconsistent. Your component is not tested and cleaned. Also, the logging is missing.
Possible recommendation: do as much as you can.

@eang0322 NIL
@MartinGaoR
Copy link
Author

Also, none of you considered the aspect of secure programming and efficiency. Most of the parts that you guys implemented are not robust as there is no validity check for inputs and no assertions was made. It would be dangerous because the current testing is still limited. @weitsang Please correct me if i am wrong or i have anything missing. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@MartinGaoR