6
\$\begingroup\$

Is there any object oriented way or design pattern I can use to make this code better? How can I avoid casting in this code? What if I add new Object like WaterTank how will it interact with fitting?

Run.java

package main;

import fittings.Fitting;
import fittings.Tap;
import bathroom.Bathroom;
import user.User;

public class Run {

    public static void main(String[] args) {

        User rutvik = new User("rutvik");

        Bathroom bathroom = new Bathroom(rutvik);   

        bathroom.getBathroomFitting(Fitting.TAP).use(rutvik);

        ((Tap)bathroom.getBathroomFitting(Fitting.TAP)).controlSpeed();

    }

}

User.java

package user;

import bathroom.BathroomUser;

public class User implements BathroomUser {

    String userName;

    public User(String userName){
        System.out.println("constructing user... "+userName);
        this.userName=userName;
    }

    @Override
    public String getUserName(){
        return userName;
    }

}

Bathroom.java

package bathroom;

import java.util.ArrayList;
import user.User;
import fittings.Fitting;
import fittings.Tap;

public class Bathroom {

    BathroomUser user;  
    ArrayList<Fitting> fittings;

    public Bathroom(User user){
        System.out.println("constructing bathroom..."); 
        System.out.println("assigning user... " + user.getUserName()+" to bathroom");
        this.user=user;
        fittings=new ArrayList<Fitting>();
        fittings.add(new Tap());
    }

    public Fitting getBathroomFitting(int fittingType){
        for(Fitting fitting:fittings){
            if(fitting.getFittingType()==fittingType){
                return fitting;
            }
        }
        return null;
    }
}

BathroomUser.java

package bathroom;

public interface BathroomUser {

    public String getUserName();

}

Fittings.java

package fittings;

import bathroom.BathroomUser;

public abstract class Fitting {

    public static final int TAP=0;
    public static final int SHOWER=1;
    public static final int SINK=2;

    public abstract void use(BathroomUser user);

    public abstract int getFittingType();

}

Tap.java

package fittings;

import bathroom.BathroomUser;

public class Tap extends Fitting {

    public Tap() {
        System.out.println("constructing TAP fitting...");
    }

    @Override
    public int getFittingType() {
        return Fitting.TAP;
    }

    @Override
    public void use(BathroomUser user) {
        System.out.println("Tap is in use... by " + user.getUserName());
    }

    public void controlSpeed(){
        System.out.println("Tap Speed Changed");
    }
}
\$\endgroup\$
1
6
\$\begingroup\$

You should be programming for interfaces.

In Bathroom instead of

ArrayList<Fitting> fittings;

use

List<Fitting> fittings;

Also you should be using Enums instead of variables. So instead of

public static final int TAP = 0;
public static final int SHOWER = 1;
public static final int SINK = 2;

you should be using

package fittings;

public enum FittingType {
   TAP, SHOWER, SINK;
}

That is a general thing - use enums instead of instance variables. But in your example you don't need that. You can do something like

public Fitting getBathroomFitting(Class<? extends Fitting> fittingType) {

    for (Fitting fitting : fittings) {
        if (fittingType.isInstance(fitting)) {
            return fitting;
        }
    }
    return null;
}

And then in your main method use

bathroom.getBathroomFitting(Tap.class).use(rutvik);

Having a enum which enumerates the sub classes of a class? Not a good idea. You can use the sub classes themselves if you want like above.

BTW in your design, if your bathroom has two taps then how are you going to use the second tap? The method just returns the first instance of that type.

About casting, you will have to cast it somewhere when you are storing as a superclass but using sub class specific things.

What if I add new Object like WaterTank how will it interact with fitting?

That all depends on what you want to represent in the code.

\$\endgroup\$
6
  • \$\begingroup\$ Can I use Map<FittingType, ArrayList<Fitting>> fittingsMap = new HashMap<Fitting.FittingType, ArrayList<Fitting>>(); for more than one fittings of same type \$\endgroup\$ – Rutvik D Mehta Feb 9 '16 at 8:39
  • \$\begingroup\$ Yes you can do that. \$\endgroup\$ – Aseem Bansal Feb 10 '16 at 3:29
  • \$\begingroup\$ Hey I have made some changes in code also solved problem for accessing more than one fittings of same type. And added WaterTank object. please review my code \$\endgroup\$ – Rutvik D Mehta Feb 10 '16 at 17:19
  • 1
    \$\begingroup\$ On code review for iterative reviews new questions are created. So please revert the changes on this question, accept answer which were helpful and create a new question with updated code and a link to this one. \$\endgroup\$ – Aseem Bansal Feb 10 '16 at 17:23
  • \$\begingroup\$ For more details search meta code review for iterative reviews \$\endgroup\$ – Aseem Bansal Feb 10 '16 at 17:28

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.