aherbert commented on code in PR #1062:
URL: https://github.com/apache/commons-lang/pull/1062#discussion_r1283652058
##########
src/main/java/org/apache/commons/lang3/time/DurationFormatUtils.java:
##########
@@ -621,22 +672,23 @@ static boolean containsTokenWithValue(final Token[]
tokens, final Object value)
private final Object value;
private int count;
- private boolean optional = false;
-
+ private int optionalIndex = -1;
Token(final Object value) {
this.value = value;
this.count = 1;
}
-
+
/**
* Wraps a token around a value. A value would be something like a 'Y'.
*
* @param value to wrap, non-null.
*/
- Token(final Object value, final boolean optional) {
+ Token(final Object value, final boolean optional, final int
optionalIndex) {
Review Comment:
You should javadoc the other parameters and why they are used.
Note: It seems odd to have the other two constructors. These are only used
in the tests. If you concur I think it sensible to remove them and add a helper
method in the test itself to create these objects using the constructor that
your code uses. IIUC the helper method can pass in the desired Object value
then call increment a number of times as appropriate.
##########
src/main/java/org/apache/commons/lang3/time/DurationFormatUtils.java:
##########
@@ -50,12 +50,27 @@
* A token character can be repeated to ensure that the field occupies a
certain minimum
* size. Values will be left-padded with 0 unless padding is disabled in the
method invocation.
* <br>
- * Tokens can be marked as optional by surrounding them with brackets [ ].
These tokens will
- * only be printed if the token value is non-zero. Any literals within
optional blocks will only be
- * printed if the nearest prior non-literal token value was non-zero.
+ * Tokens can be marked as optional by surrounding them with brackets [ ].
These tokens will
+ * only be printed if the token value is non-zero. Literals within optional
blocks will only be
+ * printed if the preceding non-literal token is non-zero. Leading optional
literals will only
+ * be printed if the following non-literal is non-zero.
+ * Multiple optional blocks can be used to group literals with the desired
token. Note that
+ * multiple optional tokens without literals can result in impossible to
understand output.
+ * (See examples below)
* <br>
+ * <table border="1">
+ * <caption>Example Output</caption>
+ *
<tr><th>pattern</th><th>Duration.ofDays(1)</th><th>Duration.ofHours(1)</th><th>Duration.ofMinutes(1)</th></tr>
Review Comment:
This is nice. For the entry with all optional formatting
`[d'd'H'h'm'm's's']` it would be useful to state what happens when the duration
is 0. E.g. If the entire format is optional and the duration is zero the result
is the empty string.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
