Add Barnsley Farn java implementation #830
Conversation
stormofice
changed the title
| return outputPoints; | ||
| } | ||
|
|
||
| public static Point matrixMultiplication(double[][] operation, double[] point) { |
There was a problem hiding this comment.
Java does not support matrix multiplication in its standard library, so this function was necessary.
Converting between Points and double Arrays is kind of unintuitive, but I think that Points being objects in general does make the code more readable.
There was a problem hiding this comment.
Maybe you could integrate the matrix multiplication logic to the Point class and remove the conversions altogether?
That would probably also involve a second constructor taking a double[] (and checking the length is 3)
There was a problem hiding this comment.
I think you should either not do the conversion at the end and return a double[], or explicitly work with a Point and return a Point.
There was a problem hiding this comment.
I've decided on keeping it consistent by using double arrays, because it is easier to work with them here.
However is checking for the array length in the constructor really necessary?
I do think that error handling would clutter up the algorithm a bit.
There was a problem hiding this comment.
Hey there, great PR overall, though I do have a few nitpicks I'd like you to address.
First off, the matrix multiplication code suspiciously lacks the loop braces. Although it's possible for single-line for loops, I don't recommend it for double for loops.
Second, I think the conversions between Point and double[] is inconsistent. Why do you want to return a Point from a double[] in the matrixMultiplication and not just do Point to Point?
Related to this, maybe adding a public Point(double[] array) could be useful for more direct conversions.
Finally, and not blocking as I'm not sure if we have a style guide decision for this, but I don't think we should teach the bad practice of letting main throw exceptions that can crash the program at any time with a potentially cryptic message.
I think we already merged Java implementations doing exactly this, so you don't have to modify that part here.
In any case, thanks for the PR :)
| return outputPoints; | ||
| } | ||
|
|
||
| public static Point matrixMultiplication(double[][] operation, double[] point) { |
There was a problem hiding this comment.
Maybe you could integrate the matrix multiplication logic to the Point class and remove the conversions altogether?
That would probably also involve a second constructor taking a double[] (and checking the length is 3)
| return outputPoints; | ||
| } | ||
|
|
||
| public static Point matrixMultiplication(double[][] operation, double[] point) { |
There was a problem hiding this comment.
I think you should either not do the conversion at the end and return a double[], or explicitly work with a Point and return a Point.
There was a problem hiding this comment.
No problem with that code.
We discussed the implementation of the matrix multiplication privately on Discord and @stormofice used the version I gave them, so I'm not sure whether I can review this version.
Amaras
added
the
Implementation
No description provided.