CODEWITHSUNDEEP
javamodel-view-controllermethodscontroller
shelholmes221
shelholmes221

Reputation: 600

Which one is better, writing another method or adding one more parameter to an existing method?

I have a method: 

public Question createQuestion(String text, Project project, User createdUser, Date createdDate)

this method is being used to create a question by the controller. Now there is no tag here in the parameters. I want to implement a functionality to add tags to a question.

To add tag I need to pass it a tagSet which can be empty as well when the user does not add tag to a question while creating the question. So, should I pass one more parameter to it and then put an if condition before adding that to the question object or should I write a separate method? 

public Question createQuestionWithTags(String text, Project project, User createdUser, Date createdDate,Set<Tag> questionTagSet)

 which will call the createQuestion and then will set the questionTagSet in the object returned by the above createQuestion() method. If I write another method then check for the empty tag will be done in the controller and if not then that check condition will be in the utility. Which approach is better?

Also, how about overloading the method in the same context?

Upvotes: 4

Views: 1530

Answers (3)

lealceldeiro
lealceldeiro

Reputation: 14978

There can be a lot of good answers to this question and, depending of your specific use case, keep in mind design patterns (and that sometimes antipatterns are justified), best practices, etc, that will make your code better.

That said, Question should have the method for adding new tags since it is the class which has the tags attribute (isn't it?). The way you implement it is up to you. It could be something like:

public class Question {
  // ...
  public void addTags(Set<Tag> questionTagSet) {
    this.tags.addAll(questionTagSet);
  //...
  }
}

This way wherever you have an object of type Question you can add tags like this:

//...
Set<Tag> tags = new HashSet<>();
Question q = new Question();
q.addTags(tags);
//...

From this point, I think there is no "best" option, but "best option for your use case". So one option would be an overloading (see Method overload below for a detailed explanation) and the other one a new method (with a different signature of course).

Method overload: One method which receives all arguments and other which receives all arguments but questionTagSet, in that one you only call the one which receives all arguments by providing a default value: null. Now, in the method which receives the questionTagSet argument you will call the Question#addTags method if the questionTagSet argument is not null. This will allow you to use the same method signature but with different arguments from the controllers. Thus you don't have to do the checking in every controller (which could be a lot), since you moved the check to only one place: the createQuestionWithTags method.

Something like this:

method with all arguments but questionTagSet

public Question createQuestionWithTags(String text, Project project, User createdUser, Date createdDate) {
  return createQuestionWithTags(text, project, createdUser, createdDate, null);
}

method with all arguments

public Question createQuestionWithTags(String text, Project project, User createdUser, Date createdDate,Set<Tag> questionTagSet) {
 Question q = new Question();
 //... some more additional logic here
 if (questionTagSet != null) { //<- logic for adding tags moved here
    q.addTags(questionTagSet);
 }
 return q;
}

This implementation can differ if you wan to do some checks with the questionTagSet argument.

Pros of this:

  • You can call the method createQuestionWithTags from different controllers in different way without worrying about the questionTagSet param:

    1. utility.createQuestionWithTags("", new Project(), new User(), new Date(), new HashSet<Tag>())

    2. utility.createQuestionWithTags("", new Project(), new User(), new Date(), null)

    3. utility.createQuestionWithTags("", new Project(), new User(), new Date())

Cons

  • Overloading can be tricky and confusing sometimes
  • When doing unit test you need to be sure which method are you testing, since the signature is almost the same.

Upvotes: 0

Eugene
Eugene

Reputation: 120978

Well to be honest, taking into consideration of how complicated overloading is in general (JLS wise), I have a hard time saying yes to it. And by hard I mean look at functional interfaces and overloading (or even search this site for related questions to see how people are pulling their hair sometimes - me included). But this is not related to lambdas only, overloading is by some people seen as too complicated and rarely needed, read this for one example.

What you have done with renaming those methods is the cleanest possible ways of achieving the task you have (unless you think of a builder pattern may be).

Upvotes: 0

Kevin Swan
Kevin Swan

Reputation: 147

This is an ideal candidate for the Builder Pattern. Declare a builder within Question, setting the parameters using the Fluent Interface Pattern, calling build() at the end to instantiate the appropriately-constructed, concrete Question instance. It would look something like:

Question.builder().withText(text).withProject(project).build();

Occurrences that require a set of tags to be specified would supply suffix the above code with a call to withTags(tags) prior to invoking build().

Upvotes: 1

Related Questions