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.

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

Code after removing certain smells #135

Closed
wants to merge 2 commits into from

Conversation

CodeBreaker2712
Copy link

I have refactored the code a little. Fixed 4 implementation smells and 5 design smells as part of my university assignment. Hope you find them useful :)

@hannesbraun
Copy link
Member

I'm sorry, but did you test your changes? Your code does not even compile... Neither is it formatted according to our style guide.

@hannesbraun
Copy link
Member

While your code compiles now, it looks like you didn't test your changes. You broke the application. And still, the code is not formatted.
Also, the .VSCodeCounter directory should not be part of this pull request. And a lot of your refactoring just adds complexity without adding any value. Or for example why move the code for intersecting a line into the Vec3f class? That's specific to a line, so the code needs to stay there. That has nothing to do with a Vec3f.

I'll close this for now. I don't see any value in these changes.

@hannesbraun hannesbraun closed this Dec 5, 2023
@CodeBreaker2712
Copy link
Author

CodeBreaker2712 commented Dec 5, 2023 via email

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