4

I've to create a library system in bluej and it has to be able to search for a book. However, I have a problem. When I try and search for a book the result is always no books available... How do I sort this so the result shows the book is available?

private List<Book> collection;

public Library()
{
    collection = new ArrayList<Book>();
}

public void addBook(Book book)
{
    collection.add(book);
}

public String titleSearch()
{
    String titleSearch = "\n ";
    for(int i = 0; i < collection.size(); i++){
        if(titleSearch.equalsIgnoreCase(collection.get(i).getTitle())){

            titleSearch = ("\n Book Avaliable");

        }else{
            titleSearch = ("\n No Books Avaliable ");
        }
    }
    return titleSearch;
}
Chris Campbell
  • 313
  • 7
  • 21
  • 2
    Two comments on coding style: using the same word as method name and local variable ... is rather bad style. In addition: using the "foreach" loop, like "for (Book book : collection) {" is better to read/maintain than the "old style" for loop. – GhostCat Mar 23 '15 at 11:43

3 Answers3

5

First, in your code is - only the last book matters, you need to break if you found a suitable book, since there is no need to check the reminders (and reset the value to "no book found") after a book is found.

for(int i = 0; i < collection.size(); i++){
    if(titleSearch.equalsIgnoreCase(collection.get(i).getTitle())){
        titleSearch = ("\n Book Avaliable");
        break; //<- added a break here, no need to go on iterating and reset titleSearch later on

    }else{
        titleSearch = ("\n No Books Avaliable ");
    }
}

The above stills fails if your collection is empty and you search for a book (that is obviously not there).
You can solve it and improve the solution by avoiding the else:

titleSearch = ("\n No Books Avaliable ");
for(int i = 0; i < collection.size(); i++){
    if(titleSearch.equalsIgnoreCase(collection.get(i).getTitle())){
        titleSearch = ("\n Book Avaliable");
        break; //<- added a break here
    }
}

This way you start pessimistic - the book is not there, and you "change your mind" if you later find it, and halts with this result.

The above still missing the title you are actually looking for, and that can be achieved by adding it as an argument and looking for it:

public String searchTitle(String titleSearch) {
    if (titleSearch == null) return "\n No Books Avaliable ";
    for(int i = 0; i < collection.size(); i++){
        if(titleSearch.equalsIgnoreCase(collection.get(i).getTitle())){
            return "\n Book Avaliable";
        }
    }
    return "\n No Books Avaliable "; //reachable only if no book found
}

And some last touch, is to use an enhanced for each loop:

public String searchTitle(String titleSearch) {
    if (titleSearch == null) return "\n No Books Avaliable ";        
    for(Book b : collection){
        if(titleSearch.equalsIgnoreCase(book.getTitle())){
            return "\n Book Avaliable";
        }
    }
    return "\n No Books Avaliable "; //reachable only if no book found
}
amit
  • 175,853
  • 27
  • 231
  • 333
  • 1
    It is very interesting that you don't mention that he's using the wrong variable to check if the title matches. He uses `titleSearch` which holds the status of his search. This search would only succeed if there is a book that contains `"\n "` (or later the current/updated status of his search). Your new approaches don't have that bug. – Tom Mar 23 '15 at 12:06
  • @Tom Thank you, let me edit it as well, for some reason I was sure it was the argument (which is missing in the metod in the first place). Also, it only equals "\n" in the firs titeration - after it, it's the value assigned to in the `else` statement – amit Mar 23 '15 at 12:07
1

if you are using Java 8 you can use stream

public String searchTitle(String titleSearch) {

    if(collection.stream().anyMatch(book->{return titleSearch.equalsIgnoreCase(book.getTitle());})){
        return "\n Book Avaliable";
    }
    else{
        return "\n No Books Avaliable";
    }
}

you can also use parallelStream() instead of stream()

pallavt
  • 86
  • 1
  • 7
-1

@amit I was using your code. When I used the

if(titleSearch.equalsIgnoreCase(Book.getTitle()))

code. I was getting an error that says "Non-static method 'getTitle()' cannot be referenced from a static context."

This is my Book Code:

class Book {
// instance variable
private String title;
private String author;
private String genre;
private String ISBN;
private boolean isCheckOut;


private static int counter;

Book(String _title) {
    this.title = _title;
    counter++;
}

/* Get Methods */

public static int getNumOfInstances() {
    return counter;
}

    String getTitle() {
    return this.title;
}

String getAuthor() {
    return this.author;
}

String getGenre() {
    return this.genre;
}

String getISBN() {
    return this.ISBN;
}

/* All Methods for setting Book */

void setAuthor(String _author) {
    this.author = _author;
}

void setGenre(String _genre) {
    this.genre = _genre;
}

void setISBN(String _isbn) {
    this.ISBN = _isbn;
}



}

Here is my Shelf Code:

import java.util.ArrayList;
import java.util.List;

public class Shelf {

// instance variable
private String name;
private String genre;

private static int counter;
public ArrayList<Book> books = new ArrayList<Book>();

Shelf(String _name) {
    this.name = _name;
    counter++;
}

static int getNumOfInstances() {
    return counter;
}

String getName() {
    return this.name;
}

String getGenre() {
    return this.genre;
}

Book getBook(int index) {
    return books.get(index);
}

int getBookSize() {
    return books.size();
}

List<Book> getBooks() {
    return this.books;
}

void setName(String _name) {
    this.name = _name;
}

void setGenre(String _genre) {
    this.genre = _genre;
}

void addBook(Book _book) {
    books.add(_book);
}


public String searchTitle(String titleSearch) {
    if (titleSearch == null) return "\n No Books Avaliable ";
    for(Book b : books){
        if(titleSearch.equalsIgnoreCase(Book.getTitle())){
            return "\n Book Avaliable";
        }
    }
    return "\n No Books Avaliable ";
}


}

By the way, the boolean is not being used at the moment.

saurish
  • 68
  • 1
  • 11