8
\$\begingroup\$

The fact is that I wanted to apply the builder pattern. But this could not be achieved, since I could not create an instance of the class with an empty constructor - this is dangerous. The goal is to make the parameter list a little shorter and more meaningful. As a result, I created a new class, the builder, which sets all the fields that are in the class in the constructor.

Why don't I want to use a builder pattern? Because the user may not use the builder at all - the required fields will be blank.

GameObject:

public abstract class GameObject {
    protected boolean isHidden;
    protected Coordinate position;
    protected int pictureWidth, pictureHeight;
    protected Object filling;

    GameObject(GameObjectBuilder gameObjectBuilder) throws IOException{
        this.position = new Coordinate(gameObjectBuilder.getPosition());
        this.pictureWidth = gameObjectBuilder.getPictureWidth();
        this.pictureHeight = gameObjectBuilder.getPictureHeight();
        this.isHidden = gameObjectBuilder.isHidden();
        this.filling = gameObjectBuilder.getFilling(); // link is passed
    }

    public void paint(Graphics gr) throws IOException{
        if(filling instanceof BufferedImage) {
            gr.drawImage((Image) filling, position.getX(), position.getY(), null);
        }
        else if(filling instanceof Color) {
            gr.setColor((Color) filling);
            gr.fillRect(position.getX(), position.getY(), pictureWidth, pictureHeight);
        } else {
            System.err.println("You forgot to add a way to render filling");
        }
    }
    private Object getFilling() {
        return filling;
    }

    int getPictureWidth() {
        return pictureWidth;
    }
    int getPictureHeight() {
        return pictureHeight;
    }
    public boolean isHidden() {
        return isHidden;
    }
    public Coordinate getPosition() {
        return position;
    }
}

GameObjectBuilder:

public class GameObjectBuilder {
    protected boolean isHidden;
    protected Coordinate position;
    protected int pictureWidth, pictureHeight;
    public Object filling;

    public GameObjectBuilder(int pictureWidth, int pictureHeight, Coordinate position, boolean isHidden, Color filling){
        this.position = new Coordinate(position);
        this.pictureWidth = pictureWidth;
        this.pictureHeight = pictureHeight;
        this.isHidden = isHidden;
        this.filling = filling; // link
    }

    public GameObjectBuilder(Coordinate position, boolean isHidden, BufferedImage filling){
        this.position = position;
        this.pictureWidth = filling.getWidth();
        this.pictureHeight = filling.getHeight();
        this.isHidden = isHidden;
        this.filling = filling; // передается ссылка
    }

    public GameObjectBuilder(GameObjectBuilder gameObject) throws IOException{
        this.position = new Coordinate(gameObject.getPosition());
        this.pictureWidth = gameObject.getPictureWidth();
        this.pictureHeight = gameObject.getPictureWidth();
        this.isHidden = gameObject.isHidden();
        this.filling = gameObject.getFilling(); //link is passed
    }

    public Object getFilling() {
        return filling;
    }

    public int getPictureWidth() {
        return pictureWidth;
    }
    public int getPictureHeight() {
        return pictureHeight;
    }
    public boolean isHidden() {
        return isHidden;
    }
    public Coordinate getPosition() {
        return position;
    }
}

and now for dessert:

MaterialGameObject:

public abstract class MaterialGameObject extends GameObject{
    private int materialHeight, materialWidth;
    private Coordinate relativeCoordinateOfStartOfFilling;

    public MaterialGameObject(MaterialGameObjectBuilder materialGameObjectBuilder) throws IOException{
        super(materialGameObjectBuilder.getGameObjectBuilder());
        this.materialHeight = materialGameObjectBuilder.getMaterialHeight();
        this.materialWidth = materialGameObjectBuilder.getMaterialWidth();
        calculateRelativeCoordinateOfStartOfFilling();
    }

    private int getMaterialWidth() {
        return materialWidth;
    }

    public int getMaterialHeight() {
        return materialHeight;
    }
    public Coordinate getRelativeCoordinateOfStartOfFilling() {
        return relativeCoordinateOfStartOfFilling;
    }
    protected abstract void calculateRelativeCoordinateOfStartOfFilling();
}

MaterialGameObjectBuilder:

public class MaterialGameObjectBuilder{
    private GameObjectBuilder gameObjectBuilder;
    private int materialHeight, materialWidth;

    public MaterialGameObjectBuilder(GameObjectBuilder gameObjectBuilder, int materialHeight, int materialWidth) {
        this.gameObjectBuilder = gameObjectBuilder; // link
        this.materialHeight = materialHeight;
        this.materialWidth = materialWidth;
    }

    public GameObjectBuilder getGameObjectBuilder(){
        return gameObjectBuilder;
    }

    public int getMaterialHeight() {
        return materialHeight;
    }
    public int getMaterialWidth() {
        return materialWidth;
    }
}

Did I choose the right path?

\$\endgroup\$
  • \$\begingroup\$ First you say you wanted to apply the builder pattern, then you say you don't. Please clarify your intentions. \$\endgroup\$ – Mast Nov 18 at 11:17
  • 1
    \$\begingroup\$ I wantED to, but then it turned out that I could not achieve some goals. And now I do not want to use it. \$\endgroup\$ – Miron Nov 18 at 11:26
4
\$\begingroup\$

This is a parameter object or argument object, which is related to the command pattern.

An example of this in the Java class library is the constraint object java.awt.LayoutManager2 (an odd name for a JDK1.00 type).

The Software Engineering StackExchange has the question Does the pattern of passing in one object instead of many parameters to a constructor have a name? [it does].

\$\endgroup\$
  • \$\begingroup\$ What? Do you mean my abstract method? \$\endgroup\$ – Miron Nov 17 at 19:10
  • \$\begingroup\$ He means Command Pattern. en.wikipedia.org/wiki/Command_pattern \$\endgroup\$ – markspace Nov 17 at 21:00
  • \$\begingroup\$ honestly, I don’t understand how this pattern relates here \$\endgroup\$ – Miron Nov 18 at 5:05
  • \$\begingroup\$ Are you saying that this is a command pattern, but instead of an action I enclose variables in it? \$\endgroup\$ – Miron Nov 18 at 5:31
  • \$\begingroup\$ This isn't really the use case of a Command pattern though.. \$\endgroup\$ – IEatBagels Nov 18 at 12:44
8
\$\begingroup\$

Yeah, this isn't a builder. I don't know if this pattern has a name, but having an intermediate data object like this which moves the complexity of multiple constructors out of the target object is a thing.

However the builder pattern doesn't require the target object to have an empty (no-argument) constructor. Example:

public class Coordinate {
  private int x;
  private int y;

  public Coordinate(int x, int y) {
     this.x = x;
     this.y = y;
  }

   // Getters...
}

public CoordinateBuilder {
  private Integer x = null;
  private Integer y = null;

  public CoordinateBuilder x(int x) {
    this.x = x;
    return this;
  }

  public CoordinateBuilder y(int y) {
    this.y = y;
    return this;
  }

  public Coordinate build() {
     if (x == null || y == null) {
        throw new IllegalStateException();
     }
     return new Coordinate(x,y);
  }
}

Coordinate a = new CoordinateBuilder().x(1).y(2).build();

Some general remarks about your code:

  • Since this isn't a builder pattern I'd rename GameObjectBuilder to something like GameObjectData.

  • Since GameObjectBuilder/Data is immutable, I'd suggest just keeping a reference of it in GameObject instead of coping the values out of it.

  • Objects should have a single "main" constructor and other constructors should call it.

  • Is it on purpose that one of the constructors doesn't call the copy constructor for position?

  • I'm not a big fan of using a plain Object.

\$\endgroup\$
  • \$\begingroup\$ it is immutable, but in the class itself, some passed variables can change. I will take this moment into account when designing. The idea with the main constructor is very useful, I will use it. I forgot to add a constructor for position. What can you suggest instead of using object? I don’t think that a list of objects of different classes, only one of which is not null, is a good idea \$\endgroup\$ – Miron Nov 17 at 19:39
  • \$\begingroup\$ @Miron If anything about its state change, then it isn't immutable. I'm not sure if I understand your comment. \$\endgroup\$ – T. Sar - Reinstate Monica Nov 18 at 20:39
  • \$\begingroup\$ By saving a link to it in a game object, do you mean aggregation? I copy the values because otherwise we will change the fields of the builder when we change the fields of the game object \$\endgroup\$ – Miron Nov 19 at 3:25
4
\$\begingroup\$

This is sure not a builder. The main characteristic of a builder is that it can build some target object.

Your "builder" is just a plain data structure. And you use it a bit like a service locator (antipattern).

The point with builders is they simplify construction of other objects. So you have some class, it has complicated constructor, maybe it accepts some abstractions and you want the builder to help you chose the correct implementations.

Rule of thumb is the target object/class is not aware of existence of the builder class. On other hand builder must be aware of the class of the objects it builds.

Also it is usualy not the target who has empty constructor, but often it is the builder. If the target had empty constructor you probably wouldn't need a builder to simplify the construction.

Also a builder must have some mutating methods. It would be hard to tell builder how to do the construction if he is unable to change anything. The builder must be able to incorporate your demands into his build plan yet before he actually uses that plan to build the target in a way that it meets your demands.

Further, the filling is where your builder can help. Dont pass just any object as filling. Create a FillingInterface, create one implementation of BufferedImage and one for Color. Add corresponding methods to your builder.

So after discussion I have changed the code sample below to show a better representation of what I mean. I also got rid of the abstract classes using decorator.

interface FillingInterface
{
  int getWidth();
  int getHeight();
  void fill(Graphics graphics, Position position);
}

class BufferedImageFilling : FillingInterface
{
  private BufferedImage image;
  BufferedImageFilling(image) {this.image = image;}

  int getWidth() {return image.getWidth();}
  int getHeight() {return image.getHeight();};

  void fill(Graphics gr, Position position) {
    gr.drawImage(this.image, position.getX(), position.getY(), null);
  }
}

class ColorFilling : FillingInterface
{
  private Color color;
  ColorFilling(Color color, int width, int height) {
    this.color = color;
    this.width = width;
    this.height = height;
  }

  int getWidth() {return this.width;}
  int getHeight() {return this.height;};

  void fill(Graphics gr, Position position) {
    gr.setColor(this.color);
    gr.fillRect(position.getX(), position.getY(), this.width, this.height);
  }

}

interface GameObjectInterface
{
  void paint(Graphics gr);
  Coordinate getPosition();
  boolean isHidden();
  int getPictureWidth();
  int getPictureHeight();
}

class GameObject : GameObjectInterface
{
  private FillingInterface filling;
  private Coordinate position;
  private boolean hidden;

  public GameObject(FillingInterface filling, Position position, boolean hidden = false) {
    this.filling = filling;
    this.position = position;
    this.hidden = hidden;
  }

  public Coordinate getPosition() {return this.position;} 
  public boolean isHidden() {return this.hidden;}
  public int getPictureWidth() {return this.filling.getWidth();}
  public int getPictureWidth() {return this.filling.getHeight();}
  public void paint(Graphics gr) {this.filling.fill(gr, this.position);}
}

class GameObjectBuilder
{
  private boolean hidden = false;
  private FillingInterface filling;
  private Coordinate position;

  public GameObjectBuilder() {}

  public void setHidden(boolean hidden) {this.hidden = hidden;}
  public void setFilling(FillingInterface filling) {this.filling = filling;}
  public void setPosition(Coordinate position) {this.position = position;}
  public GameObject build() {return new GameObject(this.filling, this.position, this.hidden);}
}

class GameObjectDecorator : GameObjectInterface
{
  private gameObject;
  public GameObjectDecorator(GameObjectInterface gameObject) {
    this.gameObject = gameObject;
  }

  public boolean isHidden() {return this.gameObject.isHidden();}
  public Coordinate getPosition() {return this.gameObject.getPosition();}
  public int getPictureWidth() {return this.gameObjet.getPictureWidth();}
  public int getPictureHeight() {return this.gameObjet.getPictureHeight();}
  public void paint(Graphics gr) {this.gameObject.paint(gr);}
}

class MaterialGameObject : GameObjectDecorator
{
  private GameObjectInterface gameObject;
  private FillingInterface materialFilling;
  public MaterialGameObject(GameObjectInterface gameObject, FillingInterface materialFilling) {
    this.gameObject = gameObject;
    this.materialFilling = materialFilling;
  }

  public void paint(Graphics gr) {
    parent.paint(gr);
    this.materialFilling.fill(gr, this.getPosition());
  }
}

class MaterialGameObjectBuilder
{
  private GameObjectInterface gameObject;
  private FillingInterface materialFilling;

  public MaterialGameObjectBuilder() {}

  public void setGameObject(GameObjectInterface gameObject) {this.gameObject = gameObject;}
  public void setMaterialFilling(FillingInterface materialFilling) {this.materialFilling = materialFilling;}
  public MaterialGameObject build() {
    return new MaterialGameObject(this.gameObject, this.materialFilling);
  }
}

Although I think at this point the builders become useless as there are now only 2-3 parameters to constructor. Should be pretty ok to use the constructors directly...

\$\endgroup\$
  • \$\begingroup\$ Comments are not for extended discussion; this conversation has been moved to chat. \$\endgroup\$ – Mathieu Guindon Nov 18 at 12:28
3
\$\begingroup\$

By no means, this is builder. It's rather a defunct clone of the original object.

A builder is something helping you to avoid too many constructors by letting you to build your object incrementally. You seem to need exactly three constructor for you object and your non-builder has them all.

Typically, a builder has a single no-args constructor and a bunch of setters for the incremental build-up. Sometimes, multiple constructors are useful for setting some "remarkable" properties upfront.

A builder typically has no getters, as you hardly ever care what's inside. When building the main object, it either passes all fields one-by-one or access them directly (the builder is pretty always in the same package and usually even in the same file, so it access private fields).

A builder has pretty always a method build(). It gets used as it's damn convenient to write things like

Person adam = Person.builder()
.name("Adam Savage")
.city("San Francisco")
.job("Mythbusters")
.job("Unchained Reaction")
.build();

A builder does make little sense without the main object being immutable. The immutability gets usually enforced by using final fields.

Fields should normally be private. I strongly recommend making everything as private as possible; lifting restrictions later is trivial, unlike the other way round.


When dealing with such objects, I prefer letting a tool take care of the boilerplate, e.g., a Builder from Lombok does a nice job. You code could be

@Value @Builder
public class GameObject {
    private final boolean isHidden;
    private final Coordinate position;
    private final int pictureWidth, pictureHeight;
    private final Object filling;

    public void paint(Graphics gr) throws IOException {
        ...
    }
}

and a builder gets generated as an nested class.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.