Refactored the rotation transformation function and examples

Sudha247 / ocaml-joy

MIT License

22 stars 14 forks source link

Refactored the rotation transformation function and examples #64

Closed nangahamandine closed 7 months ago

nangahamandine commented 10 months ago

I've updated the initial rotation transformation function to now normalize to a co-ordinate system with with the default center of (0, 0).
I also refactored the rotation transform function to use a conversion from Cartesian coordinates to polar coordinates, then rotate, then convert back to Cartesian coordinates. This therefore makes it simpler to understand.
I added a new example to test the ellipse rotation transformation and it works fine.
All rotation transformations are now displayed within the canvas
Finally, I added comments to make the implementations easier to understand.

These changes are based on the requirements of issue #60

nangahamandine commented 10 months ago

The screenshots based on the present changes can be seen below:

Screenshot (251) Screenshot (250)

nangahamandine commented 10 months ago

Nice work! @nangahamandine

I've added some comments.

Thanks for the additional comments throughout the code. Can you add them in another PR please and remove them from here? It's a cleaner commit history that way, and it also makes it easier to do better reviews.

Thank you. I'll take out the comments and update the code.

nangahamandine commented 10 months ago

Nice work! @nangahamandine

I've added some comments.

Thanks for the additional comments throughout the code. Can you add them in another PR please and remove them from here? It's a cleaner commit history that way, and it also makes it easier to do better reviews.

Thank you. I'll take out the comments and update the code.

nangahamandine commented 10 months ago

I've updated the code based on all the requested changes made by @nikochiko

I encountered some issues with refactoring into a rotate_point function and when using pattern matching, hence the delay. I succeeded to resolve all issues and everything works fine now.

Please review :)

nangahamandine commented 10 months ago

These are the attached screenshots of the rotated shapes:

Screenshot (254) Screenshot (255)

nikochiko commented 10 months ago

I encountered some issues with refactoring into a rotate_point function and when using pattern matching, hence the delay. I succeeded to resolve all issues and everything works fine now.

Awesome! I'm glad you were able to do it.

I added comments where I thought the changes weren't necessary.

At this point because our representation of rectangle / ellipse is not good enough, we aren't able to demonstrate rotation in examples properly. Once that is done we should be able to create more interesting examples!

nangahamandine commented 10 months ago

I encountered some issues with refactoring into a rotate_point function and when using pattern matching, hence the delay. I succeeded to resolve all issues and everything works fine now.

Awesome! I'm glad you were able to do it.

Thank you @nikochiko

I added comments where I thought the changes weren't necessary.

I'm not sure where, can you please reiterate so I could make the necessary changes.

At this point because our representation of rectangle / ellipse is not good enough, we aren't able to demonstrate rotation in examples properly. Once that is done we should be able to create more interesting examples!