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

ifndef update and zeroTransformationTest fix #26

Merged
merged 4 commits into from Nov 16, 2021
Merged

ifndef update and zeroTransformationTest fix #26

merged 4 commits into from Nov 16, 2021

Conversation

mbahgatTech
Copy link
Contributor

Fixed the uninitialized array of points in zeroTransformationTest.
Fixed a loop error that went over 4 vectors no matter the numOfPoints value.
Changed the ifndef define names to SCALINGINX

@cc0407 cc0407 self-requested a review November 14, 2021 17:06
Copy link
Collaborator

@cc0407 cc0407 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • The test functions are still seg-faulting. They're trying to grab points using getPoint() but there are no points in the list. It might be a better idea to manually create test points and check if your scaling function works on them, that way its independent of the amount of points in the global list
  • I'd recommend using CTRL+F and changing all instance of "vector" and "Vector" to "point" and "Point". Just so it stays consistent with the naming conventions.
  • I had commented out runScalingInXTests() inside of main.c a bit ago so the seg-fault wouldn't interfere with other groups code, please uncomment it before testing if you're not already :)

@mbahgatTech
Copy link
Contributor Author

  1. Fixed the seg faults in the first commit. (Tests seg fault fix)
  2. Changed a parameter error in the second commit (x Tests)

Copy link
Collaborator

@andrewlinington andrewlinington left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

changes have resolved issues in tests and should be good to merge

@cc0407 cc0407 merged commit ec79a38 into master Nov 16, 2021
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.

None yet

3 participants