CODEWITHSUNDEEP
javaoopdesign-patterns
sarahTheButterFly
sarahTheButterFly

Reputation: 1952

Java class interface design question

I have a class (MiddleMan) talks to a system and this talk is initialized by the MiddleMan's clients.

If a 'talk' is successfully completed, MiddleMan will generate a token and return it to its client.

The class design I have at the moment is like below:


class MiddleMan{
    private String token = null;
    private String param1;
    private String param2;
    private String amount;

    boolean isTalkSuccessful = false;

    public MiddleMan(String param1, String param2){
    this.amount = amount;
    this.param1 = param1;
    this.param2 = param2;
    }

    public String talk() throws ValidationException{

      //a private method validates param1 and param2
    validateParameters();

     //talk to backend system;
    isTalkSuccessful = talkToSystem();


    }

    Private void validateParameters() throws ValidationException {

    }

    private boolen talkToSystem(){

    }


    public boolean isTalkSuccessful(){
      return isTalkSuccessful;
    }

    public String getToken() throws Exception{
       if(isTalkSuccessful()){
          return token;
      }else{
        throw new Exception("cannot return token because talk is not  successful");
      }

     }



}

Does this look right to you? How would you refactor it?

Upvotes: 4

Views: 591

Answers (2)

Adeel Ansari
Adeel Ansari

Reputation: 39907

  1. From where the amount come into play. It's not in the list of instance variables, neither it's there in the constructor argument. You need to fix this in your snippet to avoid confusion.
  2. The validation of parameters should be done in the constructor. In case the parameters are not valid throw some appropriate exception from the constructor. talk() method should not validate, as its name is suggesting.
  3. I can see you are calling isTalkSuccessful() method in your getToken(). I am assuming you want client to check this before calling getToken() to avoid exception. Isn't it? And you have your check in place just in case client doesn't call isTalkSuccessful(). Correct? So, if avoiding the exception is the whole point, why not just don't throw any exception. Lets get rid of that public method isTalkSuccessful() and let the client call getToken(). In your getToken() method check the private field isTalkSuccessful and if it's false return null. Why not?
  4. Do you really need setToken()? Who will invoke it?

Upvotes: 3

Sujith Surendranathan
Sujith Surendranathan

Reputation: 2579

Assuming you are calling talk() first and then generating the token by calling getToken():

Don't you need to set the token variable? It is initialized with a null value, and is never modified. I guess you need to set token to whatever you want within your talk() method, after you make a successful talkToSystem() call.

Oh, and what is "amount" doing here?

this.amount = amount;

Upvotes: 0

Related Questions