4
\$\begingroup\$

I've just done an exercise on the abstract factory pattern and I want someone to tell me whether or not I followed the rules of the abstract factory pattern.

Exercise 2 (Abstract Factory: GUI based OS)

The purpose of this exercise is to provide a modelisation of a system for creating different components of a graphical user interface (buttons, menu,...) depending on the operating system (Linux, Windows, ...) to which they are intended.

The model must create components without having to know their concrete classes, to be independent of how the components evolve. The Abstract Factory pattern seems well suited.

  1. Make the UML diagram of this program. We consider that the buttons have a color, width and height, and menus just wide.

  2. Write the Java classes d'efinies by the UML diagram.

  3. Add back a new operating system (Mac OS X, for example)

yUML diagram depicting the below code

The abstract class Composant:

package Interfaces;

public abstract class Composant {
    protected double largeur ;
    protected String color = null;
    protected double hauteur ;
    
    public abstract void setColor(String C);
    public abstract void setLargeur(double L);
    public abstract void setHauteur(double H);
    
    public abstract String getColor();
    public abstract double getLargeur();
    public abstract double getHauteur();
}

The Boutton class:

package concreteClasses;

import Interfaces.Composant;

public class Boutton extends Composant{

    @Override
    public void setColor(String C) {
        color = C;
    }

    @Override
    public void setLargeur(double L) {
        largeur = L;
    }

    @Override
    public void setHauteur(double H) {
        hauteur = H;
    }

    @Override
    public String getColor() {
        return color;
    }

    @Override
    public double getLargeur() {
        // TODO Auto-generated method stub
        return largeur;
    }

    @Override
    public double getHauteur() {
        // TODO Auto-generated method stub
        return hauteur;
    }
    public String toString(){
        return "boutton color ="+color+" boutton largeur ="+largeur+" boutton     hauteur ="+hauteur;
    }
}

The Menu class:

package concreteClasses;

import Interfaces.Composant;

public class Menu extends Composant{

    @Override
    public void setColor(String C) {
        color = null;
    }

    @Override
    public void setLargeur(double L) {
        largeur = L;
    }

    @Override
    public void setHauteur(double H) {
        
    }

    @Override
    public String getColor() {
        return color;
    }

    @Override
    public double getLargeur() {
        // TODO Auto-generated method stub
        return largeur;
    }

    @Override
    public double getHauteur() {
        return hauteur;
    }
    public String toString(){
        return "Menu largeur ="+largeur;
    }
}

The abstract class AbstractFatcory:

package factoryClasses;

import Interfaces.Composant;

public abstract class AbstractFactory {
    public abstract Composant getComposant(String composant);
}

The FactoryComposant class:

package factoryClasses;

import concreteClasses.*;
import Interfaces.Composant;

public class FactoryComposant extends AbstractFactory{

    @Override
    public Composant getComposant(String composant) {
        if(composant.equalsIgnoreCase("Boutton")) return new Boutton();
        else if(composant.equalsIgnoreCase("Menu")) return new Menu();
        return null;
    }
}

The Main class:

import Interfaces.Composant;
import factoryClasses.*;

public class Main {
    public static void main (String[]args){
        
        AbstractFactory factorycomposant = new FactoryComposant();
        Composant compo = factorycomposant.getComposant("Boutton");
        compo.setColor("Red");
        compo.setHauteur(12);
        compo.setLargeur(12);
        System.out.println(compo);
        
        Composant compo2 = factorycomposant.getComposant("Menu");
        compo2.setLargeur(10);
        System.out.println(compo2);
    }
}
\$\endgroup\$
2
  • \$\begingroup\$ Here is the result: boutton color =Red boutton largeur =12.0 boutton hauteur =12.0 Menu largeur =10.0 \$\endgroup\$
    – Yassin Ousleyeh
    Commented Dec 7, 2015 at 13:57
  • \$\begingroup\$ small comment: Code Review is not intended for non-code (as the name already says) as such your UML isn't really up for review. It's mostly wrong, though. You should actually mention the types of parameters and return values in the UML... \$\endgroup\$
    – Vogel612
    Commented Dec 19, 2015 at 18:38

1 Answer 1

7
\$\begingroup\$

First of all: You got it nearly right.

One thing in your implementation deviates from the standard implementation:

@Override
public Composant getComposant(String composant) {
    if(composant.equalsIgnoreCase("Boutton")) return new Boutton();
    else if(composant.equalsIgnoreCase("Menu")) return new Menu();
    return null;
}

You parameterize your factory methods with a string. Don't do that in this case. Passing a parameter to a factory may be possible but the products should be "strong" related to each other. Everybody who wants to know what your factory can produce should see it by the method signatures provided by the abstract factory and not guess (or read in the documentation... urgh). If you want to produce two different items, create another factory method. For a factory "FactoryX" this should look like this:

@Override
public Button createBoutton() {
    return new BouttonX();
}

@Override
public Menu createMenu() {
    return new MenuX();
}

One other thing: the abstract Composant-class is irrelevant to the abstract factory in your case as far as I see. Furthermore it will bind you to a set of methods that "Menus" and "Bouttons" have in common. This would hinder you to configure special properties of "Bouttons" and "Menus". The two types may of course stay derived from Composant.

I thought about it a little bit when the use of parameters in the factory pattern may be appropriate. In the following example (in pseudo code) a LabelField or a LabelArea is returned which depends on the size of the given text.

@Override
public AbstractLabel getAbstractLabel(String text) {
    if (text.length > 120) {
        return new LabelArea(text); // multi line presentation
    } else {
        return new LabelField(text) // single line presentation
    }
}

The parameter is not technically connected to the return value but semantically. You can see the strong relationship between those return values. They were built to present text.

\$\endgroup\$
1
  • 2
    \$\begingroup\$ Well said. For some reason, factory pattern has been represented the way the OP has done it which entails to pass a string to the factory and the factory magically knows what to give you. No thought or consideration towards possible syntax errors which make that method not very robust \$\endgroup\$
    – smac89
    Commented Dec 18, 2015 at 22:40

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.