Re: svn commit: r918723 - /ofbiz/trunk/framework/base/src/org/ofbiz/base/util/string/FlexibleStringExpander.java

[Adrian Crum]

[hidden email] wrote:

> Author: doogie
> Date: Wed Mar  3 22:10:11 2010
> New Revision: 918723
>
> URL: http://svn.apache.org/viewvc?rev=918723&view=rev
> Log:
> Remove javadoc from protected methods.
>
> Modified:
>     ofbiz/trunk/framework/base/src/org/ofbiz/base/util/string/FlexibleStringExpander.java
>
> Modified: ofbiz/trunk/framework/base/src/org/ofbiz/base/util/string/FlexibleStringExpander.java
> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/base/src/org/ofbiz/base/util/string/FlexibleStringExpander.java?rev=918723&r1=918722&r2=918723&view=diff
> ==============================================================================
> --- ofbiz/trunk/framework/base/src/org/ofbiz/base/util/string/FlexibleStringExpander.java (original)
> +++ ofbiz/trunk/framework/base/src/org/ofbiz/base/util/string/FlexibleStringExpander.java Wed Mar  3 22:10:11 2010
> @@ -239,12 +239,6 @@
>          }
>      }
>  
> -    /** Parses an expression and returns an array of <code>FlexibleStringExpander</code>
> -     * instances.
> -     * @param expression The expression to be parsed
> -     * @return An array of <code>FlexibleStringExpander</code>
> -     * instances
> -     */
>      protected static FlexibleStringExpander[] getStrElems(char[] chars, int offset, int length) {
>          String expression = new String(chars, 0, length + offset);
>          int start = expression.indexOf(openBracket, offset);
> @@ -321,13 +315,6 @@
>          this.chars = chars;
>      }
>  
> -    /** Appends this object's expression result to <code>buffer</code>.
> -     *
> -     * @param buffer The buffer to append to
> -     * @param context The evaluation context
> -     * @param timeZone The time zone to be used for localization
> -     * @param locale The locale to be used for localization
> -     */
>      protected abstract Object get(Map<String, ? extends Object> context, TimeZone timeZone, Locale locale);
>  
>      private static Locale getLocale(Locale locale, Map<String, ? extends Object> context) {

Shouldn't we strive for more documentation, not less? Wouldn't that
documentation help someone understand how the class works?

[Adam Heath-2]

Adrian Crum wrote:

> [hidden email] wrote:
>> Author: doogie
>> Date: Wed Mar  3 22:10:11 2010
>> New Revision: 918723
>>
>> URL: http://svn.apache.org/viewvc?rev=918723&view=rev
>> Log:
>> Remove javadoc from protected methods.
>>
>> Modified:
>>    
>> ofbiz/trunk/framework/base/src/org/ofbiz/base/util/string/FlexibleStringExpander.java
>>
>>
>> Modified:
>> ofbiz/trunk/framework/base/src/org/ofbiz/base/util/string/FlexibleStringExpander.java
>>
>> URL:
>> http://svn.apache.org/viewvc/ofbiz/trunk/framework/base/src/org/ofbiz/base/util/string/FlexibleStringExpander.java?rev=918723&r1=918722&r2=918723&view=diff
>>
>> ==============================================================================
>>
>> ---
>> ofbiz/trunk/framework/base/src/org/ofbiz/base/util/string/FlexibleStringExpander.java
>> (original)
>> +++
>> ofbiz/trunk/framework/base/src/org/ofbiz/base/util/string/FlexibleStringExpander.java
>> Wed Mar  3 22:10:11 2010
>> @@ -239,12 +239,6 @@
>>          }
>>      }
>>  
>> -    /** Parses an expression and returns an array of
>> <code>FlexibleStringExpander</code>
>> -     * instances.
>> -     * @param expression The expression to be parsed
>> -     * @return An array of <code>FlexibleStringExpander</code>
>> -     * instances
>> -     */
>>      protected static FlexibleStringExpander[] getStrElems(char[]
>> chars, int offset, int length) {
>>          String expression = new String(chars, 0, length + offset);
>>          int start = expression.indexOf(openBracket, offset);
>> @@ -321,13 +315,6 @@
>>          this.chars = chars;
>>      }
>>  
>> -    /** Appends this object's expression result to <code>buffer</code>.
>> -     *
>> -     * @param buffer The buffer to append to
>> -     * @param context The evaluation context
>> -     * @param timeZone The time zone to be used for localization
>> -     * @param locale The locale to be used for localization
>> -     */
>>      protected abstract Object get(Map<String, ? extends Object>
>> context, TimeZone timeZone, Locale locale);
>>  
>>      private static Locale getLocale(Locale locale, Map<String, ?
>> extends Object> context) {
>
> Shouldn't we strive for more documentation, not less? Wouldn't that
> documentation help someone understand how the class works?

I suppose.  The docs were rather stale(of course, I'm the one who made
them stale).

If this class were meant to be extended by external code, then I might
tend to agree with having docs.  However, it currently doesn't support
that, as it has a hard-coded set of extension points(the nested
if/else blocks).

[Adrian Crum]

Adam Heath wrote:

> Adrian Crum wrote:
>> [hidden email] wrote:
>>> Author: doogie
>>> Date: Wed Mar  3 22:10:11 2010
>>> New Revision: 918723
>>>
>>> URL: http://svn.apache.org/viewvc?rev=918723&view=rev
>>> Log:
>>> Remove javadoc from protected methods.
>>>
>>> Modified:
>>>    
>>> ofbiz/trunk/framework/base/src/org/ofbiz/base/util/string/FlexibleStringExpander.java
>>>
>>>
>>> Modified:
>>> ofbiz/trunk/framework/base/src/org/ofbiz/base/util/string/FlexibleStringExpander.java
>>>
>>> URL:
>>> http://svn.apache.org/viewvc/ofbiz/trunk/framework/base/src/org/ofbiz/base/util/string/FlexibleStringExpander.java?rev=918723&r1=918722&r2=918723&view=diff
>>>
>>> ==============================================================================
>>>
>>> ---
>>> ofbiz/trunk/framework/base/src/org/ofbiz/base/util/string/FlexibleStringExpander.java
>>> (original)
>>> +++
>>> ofbiz/trunk/framework/base/src/org/ofbiz/base/util/string/FlexibleStringExpander.java
>>> Wed Mar  3 22:10:11 2010
>>> @@ -239,12 +239,6 @@
>>>          }
>>>      }
>>>  
>>> -    /** Parses an expression and returns an array of
>>> <code>FlexibleStringExpander</code>
>>> -     * instances.
>>> -     * @param expression The expression to be parsed
>>> -     * @return An array of <code>FlexibleStringExpander</code>
>>> -     * instances
>>> -     */
>>>      protected static FlexibleStringExpander[] getStrElems(char[]
>>> chars, int offset, int length) {
>>>          String expression = new String(chars, 0, length + offset);
>>>          int start = expression.indexOf(openBracket, offset);
>>> @@ -321,13 +315,6 @@
>>>          this.chars = chars;
>>>      }
>>>  
>>> -    /** Appends this object's expression result to <code>buffer</code>.
>>> -     *
>>> -     * @param buffer The buffer to append to
>>> -     * @param context The evaluation context
>>> -     * @param timeZone The time zone to be used for localization
>>> -     * @param locale The locale to be used for localization
>>> -     */
>>>      protected abstract Object get(Map<String, ? extends Object>
>>> context, TimeZone timeZone, Locale locale);
>>>  
>>>      private static Locale getLocale(Locale locale, Map<String, ?
>>> extends Object> context) {
>> Shouldn't we strive for more documentation, not less? Wouldn't that
>> documentation help someone understand how the class works?
>
> I suppose.  The docs were rather stale(of course, I'm the one who made
> them stale).
>
> If this class were meant to be extended by external code, then I might
> tend to agree with having docs.  However, it currently doesn't support
> that, as it has a hard-coded set of extension points(the nested
> if/else blocks).

It's not so much for someone wanting to extend it as it is for someone
wanting to work on it. Remember the questions you had about TimeDuration?

[Adam Heath-2]

Adrian Crum wrote:

> Adam Heath wrote:
>> Adrian Crum wrote:
>>> [hidden email] wrote:
>>>> Author: doogie
>>>> Date: Wed Mar  3 22:10:11 2010
>>>> New Revision: 918723
>>>>
>>>> URL: http://svn.apache.org/viewvc?rev=918723&view=rev
>>>> Log:
>>>> Remove javadoc from protected methods.
>>>>
>>>> Modified:
>>>>  
>>>> ofbiz/trunk/framework/base/src/org/ofbiz/base/util/string/FlexibleStringExpander.java
>>>>
>>>>
>>>>
>>>> Modified:
>>>> ofbiz/trunk/framework/base/src/org/ofbiz/base/util/string/FlexibleStringExpander.java
>>>>
>>>>
>>>> URL:
>>>> http://svn.apache.org/viewvc/ofbiz/trunk/framework/base/src/org/ofbiz/base/util/string/FlexibleStringExpander.java?rev=918723&r1=918722&r2=918723&view=diff
>>>>
>>>>
>>>> ==============================================================================
>>>>
>>>>
>>>> ---
>>>> ofbiz/trunk/framework/base/src/org/ofbiz/base/util/string/FlexibleStringExpander.java
>>>>
>>>> (original)
>>>> +++
>>>> ofbiz/trunk/framework/base/src/org/ofbiz/base/util/string/FlexibleStringExpander.java
>>>>
>>>> Wed Mar  3 22:10:11 2010
>>>> @@ -239,12 +239,6 @@
>>>>          }
>>>>      }
>>>>  
>>>> -    /** Parses an expression and returns an array of
>>>> <code>FlexibleStringExpander</code>
>>>> -     * instances.
>>>> -     * @param expression The expression to be parsed
>>>> -     * @return An array of <code>FlexibleStringExpander</code>
>>>> -     * instances
>>>> -     */
>>>>      protected static FlexibleStringExpander[] getStrElems(char[]
>>>> chars, int offset, int length) {
>>>>          String expression = new String(chars, 0, length + offset);
>>>>          int start = expression.indexOf(openBracket, offset);
>>>> @@ -321,13 +315,6 @@
>>>>          this.chars = chars;
>>>>      }
>>>>  
>>>> -    /** Appends this object's expression result to
>>>> <code>buffer</code>.
>>>> -     *
>>>> -     * @param buffer The buffer to append to
>>>> -     * @param context The evaluation context
>>>> -     * @param timeZone The time zone to be used for localization
>>>> -     * @param locale The locale to be used for localization
>>>> -     */
>>>>      protected abstract Object get(Map<String, ? extends Object>
>>>> context, TimeZone timeZone, Locale locale);
>>>>  
>>>>      private static Locale getLocale(Locale locale, Map<String, ?
>>>> extends Object> context) {
>>> Shouldn't we strive for more documentation, not less? Wouldn't that
>>> documentation help someone understand how the class works?
>>
>> I suppose.  The docs were rather stale(of course, I'm the one who made
>> them stale).
>>
>> If this class were meant to be extended by external code, then I might
>> tend to agree with having docs.  However, it currently doesn't support
>> that, as it has a hard-coded set of extension points(the nested
>> if/else blocks).
>
> It's not so much for someone wanting to extend it as it is for someone
> wanting to work on it. Remember the questions you had about TimeDuration?

Sure, but as javadoc?  After this series of changes, ant docs no
longer produces warnings in framework/base.