CODEWITHSUNDEEP
javasonarqube
TechCrunch
TechCrunch

Reputation: 3134

Write to static field from instance method

I have my code as below. I see 

public MyClass{

    private static DataSource dataSource = null;

    private static DataSource getDataSource(){
        if (dataSource == null) {
            try {
                dataSource = // something.
            } catch (Exception e) {
                // some exception.
            }
        }

        return dataSource;
    }

    public List doSomething(){

        // ...

        if(dataSource == null){
            dataSource = getDataSource();
        }

        dataSource.getConnection();
        // ...

    }
}

I see following message in sonar anaylsis.

Dodgy - Write to static field from instance method

This instance method writes to a static field. This is tricky to get correct if multiple instances are being manipulated, and generally bad practice.
findbugs:ST_WRITE_TO_STATIC_FROM_INSTANCE_METHOD Sep12 Reliability > Architecture

I see everything is okay in this implementation except that we are changing the static variable in doSomething method. How do we fix this ?

Upvotes: 7

Views: 18374

Answers (2)

David T.
David T.

Reputation: 23399

Not sure how your static analysis tool works but -

try writing to your value via a static setter:

private synchronized static void setDataSource(DataSource ds) {
    dataSource = ds;
}

so that you can do

   if(dataSource == null){
        setDataSource(getDataSource());
   }

Upvotes: 10

Mifmif
Mifmif

Reputation: 3190

You have to choose between two solutions , first one is in your Datasource class add getDatasource() method and synchronize the block where you instantiate your static field :

    private static DataSource getDataSource(){
     synchronize(DataSource.class){
            if (dataSource == null) {
                try {
                    dataSource = // something.
                } catch (Exception e) {
                    // some exception.
                }
            }
       }
        return dataSource;
      }

then you will just call this method from your doSomething() method

public void doSomething(){
//
DataSource dataSource=DataSource.getDataSource();
//
}

second solution is to instantiate your field at the class loading time using a static block or directly from the declaration.

static {
 dataSource = // something.
}

Upvotes: 2

Related Questions