[GitHub] [commons-configuration] kinow commented on a diff in pull request #181: CONFIGURATION-753: Interpolation Consistency

[GitBox <gi...@apache.org>]

kinow commented on code in PR #181:
URL: https://github.com/apache/commons-configuration/pull/181#discussion_r873569935


##########
src/main/java/org/apache/commons/configuration2/interpol/ConfigurationInterpolator.java:
##########
@@ -281,13 +301,34 @@ public ConfigurationInterpolator getParentInterpolator() {
      * @return the {@code StringSubstitutor} used by this object
      */
     private StringSubstitutor initSubstitutor() {
-        return new StringSubstitutor(key -> Objects.toString(resolve(key), null));
+        return new StringSubstitutor(key -> {
+            final Object value = resolve(key);
+            return value != null
+                ? stringConverter.apply(value)
+                : null;
+        });
     }
 
     /**
-     * Performs interpolation of the passed in value. If the value is of type String, this method checks whether it contains
-     * variables. If so, all variables are replaced by their current values (if possible). For non string arguments, the
-     * value is returned without changes.
+     * Performs interpolation of the passed in value. If the value is of type {@code String}, this method checks
+     * whether it contains variables. If so, all variables are replaced by their current values (if possible). For
+     * non string arguments, the value is returned without changes. In the special case where the value is a string
+     * consisting of a single variable reference, the interpolated variable value is <em>not</em> converted to a
+     * string before returning, so that callers can access the raw value. However, if the variable is part of a larger
+     * interpolated string, then the variable value is converted to a string using the configured
+     * {@link #getStringConverter() string converter}. (See the discussion on string conversion in the class
+     * documentation for more details.)
+     *
+     * <p><strong>Examples</strong></p>
+     * <p>
+     * For the following examples, assume that the default string conversion function is in place and that the
+     * variable {@code i} maps to the integer value {@code 42}.
+     * <pre>
+     *      interpolator.interpolate(1) -&gt; 1// non-string argument returned unchanged

Review Comment:
   >-&gt;
   
   You can also use the pirate symbol (that's how I remember it): `&rarr` (`→`), and `&larr;` for left-arrow :+1:  But this way is fine too (only noticed because for one second I thought it was a mistake.)



##########
src/test/java/org/apache/commons/configuration2/TestAbstractConfigurationBasicFeatures.java:
##########
@@ -200,7 +200,7 @@ private void checkGetStringArrayScalar(final Object value) {
         final BaseConfiguration config = new BaseConfiguration();
         config.addProperty(KEY_PREFIX, value);
         final String[] array = config.getStringArray(KEY_PREFIX);
-        assertEquals("Weong number of elements", 1, array.length);
+        assertEquals("Wrong number of elements", 1, array.length);

Review Comment:
   :+1: 



##########
src/main/java/org/apache/commons/configuration2/interpol/ConfigurationInterpolator.java:
##########
@@ -452,4 +511,51 @@ public void setEnableSubstitutionInVariables(final boolean f) {
     public void setParentInterpolator(final ConfigurationInterpolator parentInterpolator) {
         this.parentInterpolator = parentInterpolator;
     }
+
+    /** Class encapsulating the default logic to convert resolved variable values into strings.
+     * This class is thread-safe.
+     */
+    private static final class DefaultStringConverter implements Function<Object, String> {
+
+        /** Shared instance. */
+        static final DefaultStringConverter INSTANCE = new DefaultStringConverter();
+
+        /** {@inheritDoc} */
+        @Override
+        public String apply(final Object obj) {
+            return Objects.toString(extractSimpleValue(obj), null);
+        }
+
+        /** Attempt to extract a simple value from {@code obj} for use in string conversion.
+         * If the input represents a collection of some sort (e.g., an iterable or array),
+         * the first item from the collection is returned.
+         * @param obj input object
+         * @return extracted simple object
+         */
+        private Object extractSimpleValue(final Object obj) {
+            if (!(obj instanceof String)) {
+                if (obj instanceof Iterable) {
+                   return nextOrNull(((Iterable<?>) obj).iterator());
+                } else if (obj instanceof Iterator) {
+                    return nextOrNull((Iterator<?>) obj);
+                } else if (obj.getClass().isArray()) {
+                    return Array.getLength(obj) > 0
+                            ? Array.get(obj, 0)
+                            : null;

Review Comment:
   I think we have Lang in our list of dependencies. So we could use `ArrayUtils.get` here if you'd like. Same result, not a must-have I believe. I think it's something like
   
   ```
   ArrayUtils.get(obj, 0, null)
   ```



-- 
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: issues-unsubscribe@commons.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org