patternjavaMinor
Java method to make a string representation of a matrix
Viewed 0 times
methodmakejavarepresentationmatrixstring
Problem
I have coded this static method for converting a
It considers the width of the matrix to be equal to the length of the longest row. Now, if some matrix entries are considered missing, strings of the format
Also, rows that are empty or
MatrixUtils.java:
```
package net.coderodde.util;
/**
* This class contains static utility methods for dealing with matrices of
* {@code double} values.
*
* @author Rodion "rodde" Efremov
* @version 1.6
*/
public class MatrixUtils {
private static final int DOUBLE_FORMAT_STRINGS = 0;
private static final int STRING_FORMAT_STRINGS = 1;
/**
* The default separator length. A separator is a string of spaces used to
* separate the values in the adjacent columns on the same row.
*/
private static final int DEFAULT_SEPARATOR_LENGTH = 2;
/**
* Returns the string representation of the input matrix. The amount of
* decimals in the output equals to the longest decimal part present in
* {@code matrix}.
*
* @param matrix the matrix whose string representation to compute.
* @return a string representing the input matrix.
*/
public static String toString(final double[][] matrix) {
final int lengthOfLongestDecimalPart =
findLengthOfLongestDecimalPart(matrix);
return toString(matrix,
lengthOfLongestDecimalPart,
DEFAULT_SEPARATOR_LENGTH);
}
/**
* Returns the string representation of the input matrix. Each element in
* the output has exactly {@code decimalPartLength} decimals and the length
* of the separator is equal to {@code separatorLength} spaces.
*
* @param matrix the input matrix.
* @param decimalPartLength the amount of decimals to prin
double matrix into a nifty String.It considers the width of the matrix to be equal to the length of the longest row. Now, if some matrix entries are considered missing, strings of the format
x.xx.. are added instead of them to the overall string.Also, rows that are empty or
null are considered to be empty rows, and thus, they consist only of the x.xx.. tokens.MatrixUtils.java:
```
package net.coderodde.util;
/**
* This class contains static utility methods for dealing with matrices of
* {@code double} values.
*
* @author Rodion "rodde" Efremov
* @version 1.6
*/
public class MatrixUtils {
private static final int DOUBLE_FORMAT_STRINGS = 0;
private static final int STRING_FORMAT_STRINGS = 1;
/**
* The default separator length. A separator is a string of spaces used to
* separate the values in the adjacent columns on the same row.
*/
private static final int DEFAULT_SEPARATOR_LENGTH = 2;
/**
* Returns the string representation of the input matrix. The amount of
* decimals in the output equals to the longest decimal part present in
* {@code matrix}.
*
* @param matrix the matrix whose string representation to compute.
* @return a string representing the input matrix.
*/
public static String toString(final double[][] matrix) {
final int lengthOfLongestDecimalPart =
findLengthOfLongestDecimalPart(matrix);
return toString(matrix,
lengthOfLongestDecimalPart,
DEFAULT_SEPARATOR_LENGTH);
}
/**
* Returns the string representation of the input matrix. Each element in
* the output has exactly {@code decimalPartLength} decimals and the length
* of the separator is equal to {@code separatorLength} spaces.
*
* @param matrix the input matrix.
* @param decimalPartLength the amount of decimals to prin
Solution
First, I'd like to say that you've already done a truly fantastic job with your code. You clearly understand the value of good documentation, modular programming, and reuse of code. These are very important concepts and I can tell that you have good grasp of them already. Well done!
As for review items, I did find a few things to comment on while looking through your code. So here's what I found:
-
If you're going to use a constant to represent a fixed number of spaces, then why not just make the constant a String whose value is that number of spaces? For example
The following is how your code is written:
Now, if your intention is to create a parameter for this value later on and remove the constant then please ignore what I just said. If, however, this is truly meant to be a constant then you don't really need the extra logic to generate your separator if you simply define it differently.
-
While reading your
"Converts row to a string representing it."
which, when I read it, is confusing as the method does not return a String or anything for that matter since it's a void method.
-
Next, when I look at the method
I also notice that you're doing things like this inside of this method:
Elsewhere in your code you use
That's all I have for you. Again, you clearly have a good grasp on the fundamentals. Overall I think your code is really well written and well documented - great use of descriptive variable names, great use of Javadoc comments, great use of modular programming techniques. I do see there are no other comments in the code other than Javadoc so I just want to point out that it's a good idea to comment anything that might be tricky to remember or read easily next time you come back to it, don't be afraid to use line comments in addition to your Javadoc. Writing good code is always about making it easy for you and the next person to read and maintain, and to do that usually takes a few rounds of polishing and refactoring. You're on your way to a very polished program, good luck keep up the great work!
As for review items, I did find a few things to comment on while looking through your code. So here's what I found:
-
If you're going to use a constant to represent a fixed number of spaces, then why not just make the constant a String whose value is that number of spaces? For example
public static final String SEPARATOR=" "; The following is how your code is written:
private static final int DEFAULT_SEPARATOR_LENGTH = 2;
...
private static String getColumnSeparatorString(final int length) {
final StringBuilder sb = new StringBuilder(length);
for (int i = 0; i < length; ++i) {
sb.append(' ');
}
return sb.toString();
}Now, if your intention is to create a parameter for this value later on and remove the constant then please ignore what I just said. If, however, this is truly meant to be a constant then you don't really need the extra logic to generate your separator if you simply define it differently.
-
While reading your
rowToString method I couldn't help but think that it would easier to read if the method return type were String. From a technical standpoint this method call is fine - it works, it accomplishes your goal. However, from a readability perspective I feel like it would be better if it returned a String and you called sb.append(...) to append the returned String rather than relying on a side-effect of the method to alter the builder that is passed in. As a general rule I find it wise to avoid side effects as much as possible and when they are needed document them to the extreme because they tend to be the source of much confusion. Here your documentation says "Converts row to a string representing it."
which, when I read it, is confusing as the method does not return a String or anything for that matter since it's a void method.
-
Next, when I look at the method
createColumnFormatStrings I find myself asking: Why do both collections of strings need to be contained within a single array rather than each having their own separate structure? I suggest you could improve your code here by creating an object (or objects) that encapsulates all of this data, but holds it in two separate fields of the object so that you can easily get and set the data separately. This also solves your problem of wanting to return two things from a method instead of just one thing because you can return one object that contains all of your data, but keep the data separate and easy to understand at the same time.I also notice that you're doing things like this inside of this method:
final String doubleFormat =
"%" + (integerPartLength
+ (decimalPartLength > 0 ? 1 : 0) + decimalPartLength)
+ "."
+ decimalPartLength
+ "f";Elsewhere in your code you use
StringBuilder to do your String concatenation so I have to assume you're trying to be as efficient as possible. If this is the case I think you should definitely use StringBuilder here as well. Consider what happens if you have a very large matrix that you wish to transform into a String - it could be a lot of concatenation.That's all I have for you. Again, you clearly have a good grasp on the fundamentals. Overall I think your code is really well written and well documented - great use of descriptive variable names, great use of Javadoc comments, great use of modular programming techniques. I do see there are no other comments in the code other than Javadoc so I just want to point out that it's a good idea to comment anything that might be tricky to remember or read easily next time you come back to it, don't be afraid to use line comments in addition to your Javadoc. Writing good code is always about making it easy for you and the next person to read and maintain, and to do that usually takes a few rounds of polishing and refactoring. You're on your way to a very polished program, good luck keep up the great work!
Code Snippets
private static final int DEFAULT_SEPARATOR_LENGTH = 2;
...
private static String getColumnSeparatorString(final int length) {
final StringBuilder sb = new StringBuilder(length);
for (int i = 0; i < length; ++i) {
sb.append(' ');
}
return sb.toString();
}final String doubleFormat =
"%" + (integerPartLength
+ (decimalPartLength > 0 ? 1 : 0) + decimalPartLength)
+ "."
+ decimalPartLength
+ "f";Context
StackExchange Code Review Q#131609, answer score: 3
Revisions (0)
No revisions yet.