codenarc-developer

[Codenarc-developer] InconsistentPropertySynchronization

[Chris M. <chr...@ea...>]

Hamlet,

 

Is the intent that InconsistentPropertySynchronization cause a violation if
the class defines a synchronized getXXX() method but no matching setXXX()
method or like-named property exists? I ran into a violation for that for a
class at work.

 

Chris

Re: [Codenarc-developer] InconsistentPropertySynchronization

[Hamlet D'A. <ham...@gm...>]

That is the intent of the rule.

It is so that someone doesn't see the value of a property in the
middle of a state change.



On Sat, Jun 4, 2011 at 12:36 AM, Chris Mair <chr...@ea...> wrote:
> Hamlet,
>
>
>
> Is the intent that InconsistentPropertySynchronization cause a violation if
> the class defines a synchronized getXXX() method but no matching setXXX()
> method or like-named property exists? I ran into a violation for that for a
> class at work.
>
>
>
> Chris



-- 
Hamlet D'Arcy
ham...@gm...

Re: [Codenarc-developer] InconsistentPropertySynchronization

[Chris M. <chr...@ea...>]

>> That is the intent of the rule.
>>
>> It is so that someone doesn't see the value of a property in the middle
of a state change.

But in this case, it looks like the synchronized method is there to ensure
safe lazy-initialization of a read-only cached value. And it is a private
method. That lazy-initialization method just happens to be called
getProperties(). Is that still an "inconsistent property synchronization"?

e.g.,
class DbConfigurationProperties {
        
        private static final LOG =
Logger.getLogger(DbConfigurationProperties) 
        private static final DBCONFIGURATION_KEY =
"DbConfigurationProperties"
        
        String dbConfigurationFile
        Cache cache
        
        boolean isEnabled(Client client, Channel channel) {
                assert cache
                assert client
                assert channel
                
                def properties = getProperties()
                def propertyValue =
properties.getProperty(propertyKey(client, channel.getId()))
                propertyValue = propertyValue ?:
properties.getProperty(propertyKey(client, "*"))
                return isTrue(propertyValue)
        }
 
        private synchronized Properties getProperties() {
                Properties dbConfigurationProperties =
cache.get(DBCONFIGURATION_KEY)
                if (dbConfigurationProperties == null) {
                        dbConfigurationProperties = loadDbConfiguration()
                        cache.put(DBCONFIGURATION_KEY,
dbConfigurationProperties)
                }
                return dbConfigurationProperties
        }
                
        private String propertyKey(Client client, String channelId) {
                return "datasource.${client.id}.${channelId}"
        }
        
        private boolean isTrue(String value) {
                return value == null || value.toLowerCase() == "true"
        }
 
        private Properties loadDbConfiguration() {
                assert dbConfigurationFile
                def props = new Properties();
                try {
                        new File(dbConfigurationFile).withInputStream { 
                                inputStream -> props.load(inputStream) 
                        }
                }
                catch (FileNotFoundException e){
                        LOG.error("Error loading $dbConfigurationFile: $e")
                }
                LOG.info "Loaded DBConfiguration properties=$props"
                return props
        }
}
 
--------------------------------------------------------------
> Hamlet,
>
> Is the intent that InconsistentPropertySynchronization cause a 
> violation if the class defines a synchronized getXXX() method but no 
> matching setXXX() method or like-named property exists? I ran into a 
> violation for that for a class at work.
>
> Chris

Re: [Codenarc-developer] InconsistentPropertySynchronization

[Hamlet D. <ham...@ca...>]

Ah, this looks like a false positive. 

There is no "properties" property, just a getter. So the rule things that setProperties is being generated downstream, which it is not. 

Definitely a false positive and needs to be fixed in the rule. 


----- Original Message -----
> >> That is the intent of the rule.
> >>
> >> It is so that someone doesn't see the value of a property in the
> >> middle
> of a state change.
> 
> But in this case, it looks like the synchronized method is there to
> ensure
> safe lazy-initialization of a read-only cached value. And it is a
> private
> method. That lazy-initialization method just happens to be called
> getProperties(). Is that still an "inconsistent property
> synchronization"?
> 
> e.g.,
> class DbConfigurationProperties {
>         
>         private static final LOG =
> Logger.getLogger(DbConfigurationProperties)
>         private static final DBCONFIGURATION_KEY =
> "DbConfigurationProperties"
>         
>         String dbConfigurationFile
>         Cache cache
>         
>         boolean isEnabled(Client client, Channel channel) {
>                 assert cache
>                 assert client
>                 assert channel
>                 
>                 def properties = getProperties()
>                 def propertyValue =
> properties.getProperty(propertyKey(client, channel.getId()))
>                 propertyValue = propertyValue ?:
> properties.getProperty(propertyKey(client, "*"))
>                 return isTrue(propertyValue)
>         }
>  
>         private synchronized Properties getProperties() {
>                 Properties dbConfigurationProperties =
> cache.get(DBCONFIGURATION_KEY)
>                 if (dbConfigurationProperties == null) {
>                         dbConfigurationProperties =
>                         loadDbConfiguration()
>                         cache.put(DBCONFIGURATION_KEY,
> dbConfigurationProperties)
>                 }
>                 return dbConfigurationProperties
>         }
>                 
>         private String propertyKey(Client client, String channelId) {
>                 return "datasource.${client.id}.${channelId}"
>         }
>         
>         private boolean isTrue(String value) {
>                 return value == null || value.toLowerCase() == "true"
>         }
>  
>         private Properties loadDbConfiguration() {
>                 assert dbConfigurationFile
>                 def props = new Properties();
>                 try {
>                         new File(dbConfigurationFile).withInputStream
>                         {
>                                 inputStream ->
>                                 props.load(inputStream)
>                         }
>                 }
>                 catch (FileNotFoundException e){
>                         LOG.error("Error loading
>                         $dbConfigurationFile: $e")
>                 }
>                 LOG.info "Loaded DBConfiguration properties=$props"
>                 return props
>         }
> }
>  
> --------------------------------------------------------------
> > Hamlet,
> >
> > Is the intent that InconsistentPropertySynchronization cause a
> > violation if the class defines a synchronized getXXX() method but
> > no
> > matching setXXX() method or like-named property exists? I ran into
> > a
> > violation for that for a class at work.
> >
> > Chris
> 
> 
> ------------------------------------------------------------------------------
> Simplify data backup and recovery for your virtual environment with
> vRanger.
> Installation's a snap, and flexible recovery options mean your data
> is safe,
> secure and there when you need it. Discover what all the cheering's
> about.
> Get your free trial download today.
> http://p.sf.net/sfu/quest-dev2dev2
> _______________________________________________
> Codenarc-developer mailing list
> Cod...@li...
> https://lists.sourceforge.net/lists/listinfo/codenarc-developer
>