Making Bloch's builder pattern thread-safe: Rechecking necessary in enclosing constructor if NEVER invalid?

Question

I have recently learned Joshua Bloch's builder pattern for creating objects with many optional fields. I've been using something like it for years, but never used an inner-class until Bloch's book suggested it to me. I love it.

I understand that another thread may alter the bulider's configuration, before it's actually built (with build()), such that it may be necessary to re-validate all values in the constructor of the enclosing class. Below is an example of a builder class that optionally reverifies its data.

So my question is this: Assuming this is a robust enough design, when there are defaults for all values--knowing this class is a poor choice for using defaults--and when every set-attempt is validated, is this re-check necessary? Although it may be different, it would never be invalid. Is that correct?

(Although this design is manageable, it is certainly complicated by the potential need for re-verification. And, honestly, I never multi-thread, but I don't want to make my library unusable by people that do.)

Thank you for any advice.

/**
   <P><CODE>java ReverifyBuilderInEnclosingCnstrXmpl</CODE></P>
 **/
public class ReverifyBuilderInEnclosingCnstrXmpl  {
   public static final void main(String[] igno_red)  {

      //Don't reverify
      ReverifyBuilderInEnclosingCnstrXmpl rvbdx = new ReverifyBuilderInEnclosingCnstrXmpl.Cfg().
         name("Big Bird").age(6).build();

      System.out.println(rvbdx.sName);
      System.out.println(rvbdx.iAge);

      //Do reverify
      rvbdx = new ReverifyBuilderInEnclosingCnstrXmpl.Cfg().
         reverifyInEnclosing().
         name("Big Bird").age(6).build();
   }

   public final String sName;
   public final int    iAge;
   /**
      <P>Create a new <CODE>ReverifyBuilderInEnclosingCnstrXmpl</CODE> with defaults.</P>
   **/
   public ReverifyBuilderInEnclosingCnstrXmpl()  {
      //Does not reverify. No need.
      this(new ReverifyBuilderInEnclosingCnstrXmpl.Cfg());
   }
   private ReverifyBuilderInEnclosingCnstrXmpl(ReverifyBuilderInEnclosingCnstrXmpl.Cfg rbdx_c)  {
      sName = rbdx_c.sName;
      iAge = rbdx_c.iAge;
      ReverifyBuilderInEnclosingCnstrXmpl.Cfg.zcibValues(rbdx_c, sName, iAge, "constructor");
   }
   public static class Cfg  {
      private String  sName   = null;
      private int     iAge    = -1;
      private boolean bReVrfy = false;
      public Cfg()  {
         //Defaults
         bReVrfy = false;
         name("Broom Hilda");
         age(127);
      }
      //Self-returning configuration...START
         //No way to unset.
         public Cfg reverifyInEnclosing()  {
            bReVrfy = true;
            return  this;
         }
         public Cfg name(String s_name)  {
            zcib_name(s_name, "name");
            sName = s_name;
            return  this;
         }
         public Cfg age(int i_age)  {
            zcib_age(i_age, "age");
            iAge = i_age;
            return  this;
         }
      //Self-returning configuration...END
      //Validate config...START
         public static final void zcibValues(ReverifyBuilderInEnclosingCnstrXmpl.Cfg rbdx_c, String s_name, int i_age, String s_clgFunc)  {
            try  {
               if(!rbdx_c.bReVrfy)  {
                  return;
               }
            }  catch(NullPointerException npx)  {
               throw  new NullPointerException("zcibValues: rbdx_c");
            }
            zcib_name(s_name, s_clgFunc);
            zcib_age(i_age, s_clgFunc);
         }
         public static final void zcib_name(String s_name, String s_clgFunc)  {
            if(s_name == null  ||  s_name.length() == 0)  {
               throw  new IllegalArgumentException(s_clgFunc + ": s_name (" + s_name + ") is null or empty.");
            }
         }
         public static final void zcib_age(int i_age, String s_clgFunc)  {
            if(i_age < 0)  {
               throw  new IllegalArgumentException(s_clgFunc + ": i_age (" + i_age + ") is negative.");
            }
         }
      //Validate config...END
      public ReverifyBuilderInEnclosingCnstrXmpl build()  {
         return  (new ReverifyBuilderInEnclosingCnstrXmpl(this));
      }
   }
}

Answer 1

Firstly - the builder pattern is not inherently thread unsafe. I am not sure how you are concluding that it is. Each thread that intends to use the builder will create its own Builder object, populate it in Joshua Bloch's pragmatic and beautiful way and use it to construct the object. There are no static variables being affected anywhere in that mechanism, there is no thread unsafety unless you introduce it yourself.

Your concern about validation is - in my humble opinion - a gross pre-optimisation that produces hideously contrived and horribly bloated code. There is no reason to try to avoid validation just because you know the data is valid. Validation is almost always trivial and often takes little more that a few instructions. By bloating the class with these horrible static validation methods you are probably adding thousands of times more cpu cycles just to load this bloated code than you are saving by avoiding the validation.

Compare your contrived and bloated code with this lucid, succinct and patently correct and thread safe code and see what I mean:

public class Thing {

    public final String name;
    public final int age;

    public Thing() {
        this(new Thing.Builder());
    }

    private Thing(Thing.Builder builder) {
        name = builder.name;
        age = builder.age;
    }

    public static class Builder {

        private String name = null;
        private int age = -1;

        public Builder() {
            name("Broom Hilda");
            age(127);
        }

        public Builder name(String name) {
            if (name == null || name.length() == 0) {
                throw new IllegalArgumentException("Thing.Builder.name (" + name + ") is null or empty.");
            }
            this.name = name;
            return this;
        }

        public Builder age(int age) {
            if (age < 0) {
                throw new IllegalArgumentException("Thing.Builder.age (" + age + ") is negative.");
            }
            this.age = age;
            return this;
        }

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

Answer 2

You are misunderstanding the pattern on an architectural level: all data during construction is tied to the local thread and not to be exposed to any external handler. The very moment build is called, the now finalized set of parameters is passed to an immutable object, which then first should verify the validity of those parameters in the constructor, then either return the final object or throw an exception.

As long as you keep the builder parameters thread-local, you cannot cause any threading-issues. If you violate this rule, you should ask yourself if what you are doing is correct and/or how you could solve it in a more fine-grained way.

So if you in your example need to use the builder from different threads, the simplest and safest way is to create a new builder instance instead of doing it statically. If you worry about performance, ThreadLocal is your friend.