hat_to_the_back
hat_to_the_back

Reputation: 291

Too Many If Statements Inside Factory

I am currently developing an application with lots of different design patterns. It needs to follow good practice basically no code smells.

I am using a factory method to print out random types of objects but I have to use 3 if statements, which seems inefficient... What would happen if I wanted to print out 10 different objects? Would one just have to add more if statements is there no other way around this.

** The ultimate use of this particular method in the factory is to just return a random object (1) of ball type.

RandomGenerator ranGen = new RandomGenerator();
int randomNumber = ranGen.createRandomNumber(1,3);
if(randomNumber == 1){
     //return smallBall
}
else if(randomNumber ==2){
    //return mediumBall
}
else if(randomNumber == 3){
    //return largeBall
}

Upvotes: 3

Views: 1344

Answers (4)

sergej
sergej

Reputation: 17999

Another approach would be to apply the prototype pattern.

In the following example, we have a class named RandomBallFactory that creates random (unique) Ball instances by cloning registered prototypes.

Advantages:

  • We can add new Ball subclasses without having to change the RandomBallFactory implementation.
  • We can create objects of the same type but with different parameters.
  • We have no if-statements.

Java example:

import java.util.*;

abstract class Ball implements Cloneable {
    abstract String getName();
    public Ball clone() {
        Ball ball;
        try {
            ball = (Ball)super.clone();
        } catch (CloneNotSupportedException e) {
            ball = null;
        }
        return ball;
    }
}

class SmallBall extends Ball {
    public String getName() { return "smallBall"; }
}

class MediumBall extends Ball {
    public String getName() { return "mediumBall"; }
}

class LargeBall extends Ball {
    public String getName() { return "largeBall"; }
}

class RandomBallFactory {
    private final List<Ball> prototypes;

    public RandomBallFactory() {
        prototypes = new ArrayList<Ball>();
    }

    public void registerBall(Ball ball) {
        prototypes.add(ball);
    }

    public Ball createBall() {
        Random randomGenerator = new Random();
        Integer randomNumber = randomGenerator.nextInt(prototypes.size());
        return prototypes.get(randomNumber).clone();
    }
}

public class TestBalls {
    public static void main(String[] args) {
        RandomBallFactory randomBallFactory = new RandomBallFactory();
        randomBallFactory.registerBall(new SmallBall());
        randomBallFactory.registerBall(new MediumBall());
        randomBallFactory.registerBall(new LargeBall());

        Ball ball = randomBallFactory.createBall();
        System.out.println(ball.getName());
    }
}

C++ example:

#include <iostream>
#include <vector>
#include <memory>
#include <cstdlib>
#include <ctime>

class Ball {
public:
    Ball() { std::cout << __func__ << std::endl; }
    Ball(Ball& other) { std::cout << __func__ << " copy from " << other.getName() << std::endl; }
    virtual ~Ball() { std::cout << __func__  << std::endl; }
    virtual std::string getName() = 0;
    virtual Ball* clone() = 0;
};

class SmallBall : public Ball {
public:
    std::string getName() { return "smallBall"; }
    Ball* clone() { return new SmallBall(*this); }
};

class MediumBall : public Ball {
public:
    std::string getName() { return "mediumBall"; }
    Ball* clone() { return new MediumBall(*this); }
};

class LargeBall : public Ball {
public:
    std::string getName() { return "largeBall"; }
    Ball* clone() { return new LargeBall(*this); }
};

class RandomBallFactory {
private:
    std::vector<std::shared_ptr<Ball> > prototypes;

public:
    void registerBall(std::shared_ptr<Ball> ball_ptr) {
        prototypes.push_back(ball_ptr);
    }

    std::shared_ptr<Ball> createBall() {
        int randomNumber = std::rand() % prototypes.size();
        return std::shared_ptr<Ball>(prototypes.at(randomNumber)->clone());
    }
};

int main(void) {
    std::srand(std::time(0));
    RandomBallFactory randomBallFactory;

    std::shared_ptr<Ball> sb_ptr(std::make_shared<SmallBall>());
    std::shared_ptr<Ball> mb_ptr(std::make_shared<MediumBall>());
    std::shared_ptr<Ball> lb_ptr(std::make_shared<LargeBall>());

    randomBallFactory.registerBall(sb_ptr);
    randomBallFactory.registerBall(mb_ptr);
    randomBallFactory.registerBall(lb_ptr);

    std::shared_ptr<Ball> ball_ptr(randomBallFactory.createBall());
    std::cout << "random Ball is: " << ball_ptr->getName() << std::endl;
}

Upvotes: 3

John Bollinger
John Bollinger

Reputation: 180141

You have at least two likely techniques available to you to provide for random generation of objects without hard-coding a fixed set of alternatives:

  1. Randomization of constructor / factory-method parameters, and
  2. Use of a randomly-selected builder object from a collection of such objects maintained by the factory.

I'll focus on the latter. The suggestion to return a random element from a collection of pre-built ones is a special case, wherein the builder objects trivially provide themselves as the generated object. A more general form might resemble this:

interface Builder<T> {
    T createObject();
}

class Factory<T> {
    private final List<Builder<? extends T>> builders = new ArrayList<>();
    private final RandomGenerator ranGen = new RandomGenerator();

    T createRandomObject() {
        int randomNumber = ranGen.createRandomNumber(0, builders.size() - 1);

        return builders.get(randomNumber).createObject();
    }

    // Not shown: mechanisms for managing the available Builder objects
}

Upvotes: 1

sergej
sergej

Reputation: 17999

You can use a Map, something like this (assuming that SmallBall and others are subclasses of Ball):

Map<Integer, Ball> balls = new HashMap<Integer, Ball>();

balls.put(1, new SmallBall());
balls.put(2, new MediumBall());
balls.put(3, new LargeBall());

RandomGenerator ranGen = new RandomGenerator();
Integer randomNumber = ranGen.createRandomNumber(1, balls.size());

return balls.get(randomNumber);

Note: In this example, the factory method would always return a reference to one of the three instances, no new objects are created.

If you want multiple unique instances, put concrete Ball-factories into the map:

Map<Integer, BallFactory> ballFactories = new HashMap<Integer, BallFactory>();

ballFactories.put(1, new SmallBallFactory());
ballFactories.put(2, new MediumBallFactory());
ballFactories.put(3, new LargeBallFactory());

RandomGenerator ranGen = new RandomGenerator();
Integer randomNumber = ranGen.createRandomNumber(1, balls.size());

return ballFactories.get(randomNumber).createBall();

Upvotes: 3

Paul Evans
Paul Evans

Reputation: 27567

The simplest solution is to use a switch statement, something like:

int randomNumber = ranGen.createRandomNumber(1,3);
switch (randomNumber) {
    case 1:
        // return smallBall
        break;
    case 2:
        // return mediumBall
        break;
    case 3:
        // return largeBall
        break;
    default:
        // handle non-expected value
        break;
}

Upvotes: -1

Related Questions