Skip to content

Conversation

@Sumamasonia
Copy link

Hi,

I noticed this issue (#811) was inactive for a while, so I went ahead and implemented the fix.

Changes:
Added default print() method to PMatrix interface.

Added a default method to print matrix data.
@catilac catilac requested a review from Stefterv December 10, 2025 13:19
@catilac catilac self-requested a review January 29, 2026 13:06
Copy link
Collaborator

@catilac catilac left a comment

Choose a reason for hiding this comment

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

Hi @Sumamasonia Thank you so much! So my review is this:

The solution is a little simpler, than what you have, but it's the right idea. Since we're dealing with the interface of PMatrix java's polymorphism allows you to simply declare public void print(); and it will know what to do. Once you make this change i'll merge it in!

@catilac catilac removed the request for review from Stefterv January 29, 2026 13:13
@Sumamasonia
Copy link
Author

Hi @catilac, I have finished the updates. Added the print() declaration to the PMatrix interface and implemented formatted toString() methods in both PMatrix2D and PMatrix3D. This makes the solution simpler and polymorphic as suggested. Ready for your review!

@catilac
Copy link
Collaborator

catilac commented Jan 29, 2026

@Sumamasonia Hi! Can you help me understand the changes you made in PMatrix2d.java and PMatrix3D.java? I don't think you need to make any changes to the implementations there.

@Sumamasonia
Copy link
Author

Hi @catilac! The reason I updated the implementations is to leverage polymorphism properly as you suggested.

Previously, both classes had their own independent print() logic. By adding public void print() to the PMatrix interface, I added the @OverRide annotation to the existing methods in PMatrix2D and PMatrix3D to explicitly link them.

Also, I refactored the formatting logic into toString(). This makes the code cleaner and allows developers to use System.out.println(matrix) as well as matrix.print(), with both using the same formatting logic. Let me know if you’d prefer me to revert the implementation changes and keep the original print() methods as they were!

@catilac
Copy link
Collaborator

catilac commented Jan 29, 2026

@Sumamasonia ohhhh I see what you've done. That's really cool. Sorry I didn't understand it

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