Java - Missing Static Method In Non Instantiatable Class

Question

I have 'builder' class and Sonar gives the following warning:

Missing Static Method In Non Instantiatable Class
pmd : MissingStaticMethodInNonInstantiatableClass
A class that has private constructors and does not have any static methods or fields cannot be used

How would I refactor this class to satisfy the above check? I'm scratching my head because I do use that class.

Sample use:

ViewBuilder vb = new ViewBuilder.Builder()
    .modelPart(CONTENT_PAGE, contentPageDao.get(id))
    .modelPart("navigation_sections", navigationSectionDao.list() )
    .modelPart("available_tags", tagDao.list() )
    .modelPart("available_albums", albumDao.list() )
    .section(CONTENT_PAGE)
    .page("index")
    .element("form")
    .action("/admin/content_page/save/" + id + ".html")
    .build();

Class itself:

import java.util.HashMap;
import java.util.Map;


 public final class ViewBuilder {

    private static final String ADMIN_LAYOUT = "admin/layout";
    private String layout = ADMIN_LAYOUT;
    private String section = "";

  private static Map<String, Object> viewParts = new HashMap<String, Object>();

  /**
   * @return the layout
   */ public String getLayout() {
    return layout;
  }

  /**
   * @param layout the layout to set
   */ public void setLayout(String layout) {
    this.layout = layout;
  }

  /**
   * @return the section
   */ public String getSection() {
    return section;
  }

  /**
   * @param section the section to set
   */ public void setSection(String section) {
    this.section = section;
  }

  public static class Builder{

    // required params
    private String layout;
    private String section;

    // optional params
    private Map<String, Object > viewParts = new HashMap<String, Object >();

    public Builder(){

        this.layout = ADMIN_LAYOUT;
        viewParts.put("layout", ADMIN_LAYOUT);

    }
    public Builder( String layout ){

      if( layout != null ){
        this.layout = layout;
        viewParts.put("layout", layout );
      } else {
        this.layout = ADMIN_LAYOUT;
        viewParts.put("layout", ADMIN_LAYOUT);
      }

    }// constructor
    public Builder modelPart( String val, Object o ){
      this.viewParts.put(val, o );
      return this;
    }

    public Builder action( String val ){
      this.viewParts.put("action", val);
      return this;
    }
    public Builder element( String val ){
      this.viewParts.put("element", val);
      return this;
    }
    public Builder section( String val ){
      this.section = val;
      this.viewParts.put("section", val);
      return this;
    }
    public Builder page( String val ){
      this.viewParts.put("page", val);
      return this;
    }
    public Builder layout( String val ){
      this.layout = val;
      return this;
    }

    public ViewBuilder build( ){
      return new ViewBuilder( this );
    }


  }// Builder

    private ViewBuilder(Builder builder){
      this.section = builder.section;
      this.layout = builder.layout;

      viewParts = builder.viewParts;
    }

  /**
   * Get the value of viewParts
   *
   * @return the value of viewParts
   */
  public Map<String, Object> getViewParts() { return viewParts; }

  /**
   * Set the value of viewParts
   *
   * @param viewParts new value of viewParts
   */
  public void setViewParts(Map<String, Object> viewParts) { this.viewParts = viewParts; }
}

score 5 · Answer 1 · answered Mar 25 '15 at 17:32

5

The SonarQube PMD rule needs to be updated to take into account the accessor in the static nested builder class.

There is a PMD issue created for this but it looks abandoned: http://sourceforge.net/p/pmd/bugs/955/

I disagree with "refactoring" to get rid of this sonar violation, the builder pattern is well known and very useful when creating complex objects. See "Effective Java" Chapter 2, item 2. http://www.informit.com/articles/article.aspx?p=1216151&seqNum=2

answered Mar 25 '15 at 17:32

bmg

153
2
8

Thank goodness someone has spoken to the value of the builder pattern. The problem is *not* this person's code. It's that sonar is not "smart" enough to understand the private constructor IS being called by an inner classes "build()" function and hence Sonar should then realize that functionality is public and quit complaining. +brownie points for referencing Josh Bloch's book. Anyone who thinks this code provides no value as written owe's it to themselves to pick up Effective Java and give it a read. – Russ Dec 10 '15 at 15:53

score 1 · Answer 2 · answered Apr 04 '18 at 15:38

An alternative way to satisfy this check is to make the constructors package-private rather than private. An added benefit of this is that the compiler will no longer need to generate an additional synthetic constructor with package-private access for the inner class, ref an answer to a question about private methods in inner classes.

If you feel strongly about keeping the constructors private, to prevent even other classes in the same package from creating instances without using Builder.build(), you should be able to suppress the Sonar issue with:

// private constructor to enforce the use of Builder.build()
@SuppressWarnings("pmd:MissingStaticMethodInNonInstantiatableClass")

score 0 · Accepted Answer · answered Jan 18 '13 at 22:55

0

Given that your nested Builder class is public and static, I really see little benefit in it being nested at all. I would recommend breaking it out into its own top level class. Of course, thats not why PMD is complaining. Your outer class, ViewBuilder, has no public constructors, and no static methods. As it exists right now, its a fairly useless 'namespace wrapper' around your Builder class. Either add a constructor to it, or get rid of it altogether.

answered Jan 18 '13 at 22:55

Perception

79,279
19
185
195

+1. "public and static, I really see little benefit in it being nested at all" – kosa Jan 18 '13 at 22:57
3

This is exactly how you implement a "builder pattern". If you think there is "little value" to it I recommend you do some research on the point of the builder pattern. In short, it's to be able to reliably create an object in a guaranteed state. The private class is utilized to make sure the outer classes constructor is ONLY called after the build() function is called and also provides a hookpoint to do validation against the variables passed in. Now you can do a sanity check seeing if you had the "required" values passed in or not. See response by bmg for more info. – Russ Dec 10 '15 at 15:49

Java - Missing Static Method In Non Instantiatable Class

3 Answers3