Dawid
Dawid

Reputation: 53

Refactoring the code of many similar methods

Hi I have many similar methods in my code as below and maybe I will have more in the future.

public void getParticularBook(String nameOfBook){
    String bookDetails = "";
    Iterator<Book> iterator = allBooks.iterator();
    while(iterator.hasNext()) {
        Book b = iterator.next();
        if(b.getTitle().equalsIgnoreCase(nameOfBook)){
            bookDetails = b.toString();
        }
    }
    System.out.println(bookDetails);
}

public void getBooksDataOnRange(int from, int to){
    String bookDetails = "";
    Iterator<Book> iterator = allBooks.iterator();
    while(iterator.hasNext()) {
        Book b = iterator.next();
        if(b.getIssueYear() >= from && b.getIssueYear() <= to){
            bookDetails = b.toString();
        }
    }

    if(bookDetails.isEmpty()){
        System.out.println("No books in range of: " + from + "-" + to);
    }
    System.out.println(bookDetails);
}

public void getBooksDataOnType(String type){
    String bookDetails = "";
    Iterator<Book> iterator = allBooks.iterator();
    while(iterator.hasNext()) {
        Book b = iterator.next();
        if(b.getType().equalsIgnoreCase(type)){
            bookDetails = b.toString();
        }
    }

    if(bookDetails.isEmpty()){
        System.out.println("No books of type: " + type);
    }
    System.out.println(bookDetails);
}

Above methods are some kind of filters which returns data based on e.g. type of the book, issue date of the book.

And the question is, if it is possible to refactor the code of all that kind of methods? Or maybe better to follow above scheme? Thanks for the answers

Upvotes: 2

Views: 95

Answers (3)

Veselin Davidov
Veselin Davidov

Reputation: 7081

There are many different options to do it. And the basic idea is to separate the iteration on the books from the testing if a book matches your search.

For example some kind of command pattern and passing a method that compares two books:

public void getBooks(Book testAgainst, Comparator<Book> comparator){
iterate
comparator.compare(bookFromIterator, testAgainst);
print result if matches  
}

Then you can pass different comparators to that method - for example title comparator, years comparator etc.

Another (and a better) way to do it is if you look at how Hibernate queries data. Create something like "BookSearchCriteria" class. Something like:

class BookSearchCriteria {
  int fromYear;
  int toYear;
  String title;

  public boolean matches(Book book){
        ... test for title, test for year etc...
   }
}

And your method will become:

public void getBooks(BookSearchCriteria  criteria){
iterator
....
if(criteria.matches(book)) then do stuff
}

This would be a better example because you can extend that criteria in many ways - for example combining properties, making it AND or OR criteria etc.

Upvotes: 0

Ashwin K Kumar
Ashwin K Kumar

Reputation: 113

There are multiple solutions to your problem.

For the getParticularBook() method, you can have a separate HashMap<String,Book>, so that you need not iterate through all the books every time. This will work only if there are no duplicates.

For the getBooksDataOnRange() method, you can have a HashMap<Integer, ArrayList<Book>> where the integer will be the year and the ArrayList will contain the list of books published in that year. If memory is an issue here, you can have a HashMap<Integer, ArrayList<String>> where instead of storing the book object, you will be storing the name of the book. You can get the book object from the previous HashMap.

For the getBooksDataOnType() method, I guess the actual requirement is to get the data of all the books of the given type. Here I would suggest a HashMap<String, ArrayList<String>> if a single book can have multiple types. If not, and if memory is not an issue for you, then you can have a HashMap<String, ArrayList<Book>> for this.

Here are the Java Documentation links to HashMap and ArrayList. This would help you with the implementation.

HashMap: https://docs.oracle.com/javase/8/docs/api/java/util/HashMap.html

ArrayList: https://docs.oracle.com/javase/8/docs/api/java/util/ArrayList.html

Upvotes: 0

Sweeper
Sweeper

Reputation: 274565

You can create a method called getBooksWithPredicate:

public static void getBooksWithPredicate(Predicate<Book> predicate, String errorMessage) {
    String bookDetails = "";
    Iterator<Book> iterator = allBooks.iterator();
    while(iterator.hasNext()) {
        Book b = iterator.next();
        if(predicate.test(b)){
            bookDetails = b.toString();
        }
    }

    if(bookDetails.isEmpty()){
        System.out.println(errorMessage);
    }
    System.out.println(bookDetails);
}

This is basically a generalization of all three of the methods shown. The three methods can then be implemented by calling this method:

public void getParticularBook(String nameOfBook){
    getBooksWithPredicate(b -> b.getTitle().equalsIgnoreCase(nameOfBook), "");
}

public void getBooksDataOnRange(int from, int to){
    getBooksWithPredicate(b -> b.getIssueYear() >= from && b.getIssueYear() <= to, "No books in range of: " + from + "-" + to);
}

public void getBooksDataOnType(String type){
    getBooksWithPredicate(b -> b.getType().equalsIgnoreCase(type), "No books of type: " + type);
}

Upvotes: 3

Related Questions