commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From sebb <seb...@gmail.com>
Subject Re: svn commit: r1528612 - /commons/sandbox/monitoring/trunk/reporting/src/main/java/org/apache/commons/monitoring/reporting/web/plugin/PluginRepository.java
Date Fri, 04 Oct 2013 18:29:19 GMT
On 4 October 2013 18:04, Jean-Louis MONTEIRO <jeanouii@gmail.com> wrote:
> Apologize for the late answer.
> Not sure to understand the purpose of the request.
> Could you detail cause it's not used anywhere else?

The code should not contain 'magic' strings (or numbers for that matter).

All fixed strings should be documented as to their purpose and derivation.
The conventional way to do this is via a constant with appropriate Javadoc.

In other words, why is the suffix ".activated" ?
Why not ".alive" or ".on" or ".randomString"?

> JLouis
>
>
> 2013/10/3 sebb <sebbaz@gmail.com>
>
>> On 2 October 2013 21:12,  <jlmonteiro@apache.org> wrote:
>> > Author: jlmonteiro
>> > Date: Wed Oct  2 20:12:29 2013
>> > New Revision: 1528612
>> >
>> > URL: http://svn.apache.org/r1528612
>> > Log:
>> > Fixing configuration property typo
>> >
>> > Modified:
>> >
>> commons/sandbox/monitoring/trunk/reporting/src/main/java/org/apache/commons/monitoring/reporting/web/plugin/PluginRepository.java
>> >
>> > Modified:
>> commons/sandbox/monitoring/trunk/reporting/src/main/java/org/apache/commons/monitoring/reporting/web/plugin/PluginRepository.java
>> > URL:
>> http://svn.apache.org/viewvc/commons/sandbox/monitoring/trunk/reporting/src/main/java/org/apache/commons/monitoring/reporting/web/plugin/PluginRepository.java?rev=1528612&r1=1528611&r2=1528612&view=diff
>> >
>> ==============================================================================
>> > ---
>> commons/sandbox/monitoring/trunk/reporting/src/main/java/org/apache/commons/monitoring/reporting/web/plugin/PluginRepository.java
>> (original)
>> > +++
>> commons/sandbox/monitoring/trunk/reporting/src/main/java/org/apache/commons/monitoring/reporting/web/plugin/PluginRepository.java
>> Wed Oct  2 20:12:29 2013
>> > @@ -35,7 +35,7 @@ public final class PluginRepository {
>> >              if (name == null) {
>> >                  throw new IllegalArgumentException("plugin name can't
>> be null");
>> >              }
>> > -            if (!Configuration.is(name + "activated", true)) {
>> > +            if (!Configuration.is(name + ".activated", true)) {
>>
>> I assume that this string is used elsewhere within monitoring?
>> If so, it should be defined once as a String constant (with Javadoc)
>> and used throughout.
>> Or there could be a method to convert a name by appending the suffix.
>>
>> >                  continue;
>> >              }
>> >
>> >
>> >
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>> For additional commands, e-mail: dev-help@commons.apache.org
>>
>>
>
>
> --
> Jean-Louis

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Mime
View raw message