Library system in Java with OOP

Question

I am practicing designing OOP projects. And this is my first OOP design.

I need some reviews for the code and the design please.

Here is what they call UML.

The code.

import java.util.HashMap;

public class Library {
    private static HashMap<Book, Boolean> store = new HashMap<Book, Boolean>();
    public static HashMap<Book, Boolean> getStore() {
        return store;
    }
    public static boolean isBookExist(Book book) {
        if(store.containsKey(book))
            return store.get(book);
        return false;
    }
}

public class Book {
    private String name, author;
    Book(String name, String author) {
        this.name = name;
        this.author = author;
    }
    String getName() {
        return name;
    }
    String getAuthor() {
        return author;
    }
}

public class Action {
    void readBook(Book book) {
        if(Library.isBookExist(book))
            System.out.println("An user is reading book: \"" + book.getName() + "\" for the author: " + book.getAuthor());
        else
            System.out.println("Book \"" + book.getName() + "\" for the author: " + book.getAuthor() + " is not exist");
    }
}

public class AdminAction extends Action {
    protected void addBook(Book book) {
        if(Library.isBookExist(book)) {
            System.out.println("This book is already in the library");
            return;
        }
        Library.getStore().put(book, true);
        System.out.println("Book: \"" + book.getName() + "\" for the author: " + book.getAuthor() + "is added to the library");
    }

    protected void removeBook(Book book) {
        if(!Library.isBookExist(book)) {
            System.out.println("This book is not in the library");
            return;
        }
        Library.getStore().remove(book);
        System.out.println("Book: \"" + book.getName() + "\" for the author: " + book.getAuthor() + "is removed from the library");
    }
}

public class User extends Action {
    private String name;
    User (String name) {
        this.name = name;
    }
    public String getName() {
        return name;
    }
    public void changeName(String newName) {
        this.name = name;
    }
}

public class Admin extends AdminAction {
    private String name;
    Admin(String name) {
        this.name = name;
    }
    public String getName() {
        return name;
    }
    public void changeName(String newName) {
        this.name = name;
    }
}

public class LibrarySystem {
    public static void main(String[] args) {
        Admin admin = new Admin("Omar");
        Admin admin2 = new Admin("Ahmed");
        User user = new User("user1");
        User user2 = new User("user2");
        Book book = new Book("Introduction to algorithms", "Omar");
        Book book2 = new Book("Clean Code", "Ghada");
        Book book3 = new Book("Clean Arch", "Waled");
        Book book4 = new Book("Head First", "Hend");
        user.readBook(book);
        admin2.addBook(book);
        admin2.addBook(book2);
        admin2.addBook(book3);
        admin.addBook(book4);
        user2.readBook(book);
        user.readBook(book);
        user.readBook(book2);
        admin2.removeBook(book);
        user.readBook(book);
        user2.readBook(book);
        user.readBook(book4);
    }
}

Reinderien · Accepted Answer · 2022-09-09 16:21:48Z

It is not a good idea to hold a map of books to booleans in this case. Why would we want to store a false when we can just store nothing at all? Use a set instead.

There's static abuse here. Library needs to be instantiated and should not contain statics.

The members of Library should be immutable (final).

Book is simple enough, and should be immutable, so should be a record.

isBookExist is better-phrased hasBook.

Grammar: An user -> A user.

You need to make better use of string formatting favoured over string concatenation.

Action as a class doesn't make a lot of sense, nor do its verbs addBook etc. Since these verbs mutate the library, they should be written in the library class. As a rudimentary form of access control, if you want only administrators to be able to add and remove books, require that the method call accept an instance of an administrative user.

Library administration systems don't technically care whether their clients read the books; only whether the books have been loaned - so some terminology needs to change.

Most errors should raise exceptions rather than print, on the inside of business logic; then at the point of call decide whether to let the exceptions fall through or catch and print them.

A User allowing a name to be get and set through boilerplate accessor methods is no functionally different than simply having a public member variable, so do that.

Suggested

import java.util.HashMap;
import java.util.HashSet;

public class Main {
    public static class LibraryException extends RuntimeException {
        public LibraryException(String message) { super(message); }
    }

    public record Book(String name, String author) {
        @Override
        public String toString() {
            return "\"%s\" for the author: %s".formatted(name, author);
        }
    }

    public static class User {
        public String name;

        public User(String name) { this.name = name; }

        @Override
        public String toString() { return name; }
    }

    public static class Admin extends User {
        public Admin(String name) { super(name); }
    }

    public static class Library {
        private final HashSet<Book> store = new HashSet<>();
        private final HashMap<Book, User> loans = new HashMap<>();

        public boolean hasBook(Book book) {
            return store.contains(book);
        }

        public void needBook(Book book) {
            if (!hasBook(book))
                throw new LibraryException("Book %s is not in the library".formatted(book));
        }

        public void addBook(Admin user, Book book) {
            if (!store.add(book))
                throw new LibraryException("Book %s is already in the library".formatted(book));
            System.out.printf("Book %s is added to the library%n", book);
        }

        public void removeBook(Admin user, Book book) {
            needBook(book);
            loans.remove(book);
            store.remove(book);
            System.out.printf("Book %s is removed from the library%n", book);
        }

        public void loanBook(User user, Book book) {
            needBook(book);
            if (loans.containsKey(book))
                throw new LibraryException("Another user has loaned book %s".formatted(book));
            loans.put(book, user);
            System.out.printf("User %s has loaned book %s%n", user, book);
        }
    }

    public static void main(String[] args) {
        Admin admin = new Admin("Omar");
        Admin admin2 = new Admin("Ahmed");
        User user = new User("user1");
        User user2 = new User("user2");
        Book book = new Book("Introduction to algorithms", "Omar");
        Book book2 = new Book("Clean Code", "Ghada");
        Book book3 = new Book("Clean Arch", "Waled");
        Book book4 = new Book("Head First", "Hend");

        Library library = new Library();

        try { library.loanBook(user, book); }
        catch (LibraryException e) { System.out.println(e); }

        library.addBook(admin2, book);
        library.addBook(admin2, book2);
        library.addBook(admin2, book3);
        library.addBook(admin, book4);
        library.loanBook(user2, book);

        try { library.loanBook(user, book);}
        catch (LibraryException e) { System.out.println(e); }

        library.loanBook(user, book2);
        library.removeBook(admin2, book);

        try { library.loanBook(user, book); }
        catch (LibraryException e) { System.out.println(e); }

        try { library.loanBook(user2, book); }
        catch (LibraryException e) { System.out.println(e); }
        
        library.loanBook(user, book4);
    }
}

Output

Main$LibraryException: Book "Introduction to algorithms" for the author: Omar is not in the library
Book "Introduction to algorithms" for the author: Omar is added to the library
Book "Clean Code" for the author: Ghada is added to the library
Book "Clean Arch" for the author: Waled is added to the library
Book "Head First" for the author: Hend is added to the library
User user2 has loaned book "Introduction to algorithms" for the author: Omar
Main$LibraryException: Another user has loaned book "Introduction to algorithms" for the author: Omar
User user1 has loaned book "Clean Code" for the author: Ghada
Book "Introduction to algorithms" for the author: Omar is removed from the library
Main$LibraryException: Book "Introduction to algorithms" for the author: Omar is not in the library
Main$LibraryException: Book "Introduction to algorithms" for the author: Omar is not in the library
User user1 has loaned book "Head First" for the author: Hend

"A User allowing a name to be get and set through boilerplate accessor methods is no functionally different than simply having a public member variable, so do that." I did that to apply the concept of encapsulation. Maybe in the future, we will make some limitations on how the name will be changed. So it will be more convenient to apply it in our setter function instead of rewriting a lot of other codes that call the variable name directly. — Omar_Hafez, Commented Sep 10, 2022 at 0:26
Boilerplate accessors are no more encapsulated than a bare member variable — Reinderien, Commented Sep 10, 2022 at 3:30

Stack Exchange Network

Library system in Java with OOP

1 Answer 1

Suggested

Output

Your Answer

Hot Network Questions

Library system in Java with OOP

1 Answer 1

Suggested

Output

Your Answer

Sign up or log in

Post as a guest

Related

Hot Network Questions