Online Book Reader Object Oriented Design

Question

I have designed and Online Book Reader System. You can find the classes below. I would be appreciated for the valuable reviews.

Assumptions: "Online Book Reader System" is a system includes online books and serve their customers to read books online. User should be login to reach their accounts. System can include many users and many books. Multiple users can access the same book. User can add any books to their reading list. After users login to system they can start to read any book they want and also can proceed with the latest page they have resumed.

Especially considering;

Multi-threading / SOLID principles / Design patterns

Book.java

package OOPDesign.onlineBookReaderSystem;

import java.util.ArrayList;

public abstract class Book {

    long bookId;
    String name;
    String category;
    String author;
    int pageCount;

    ArrayList<User> readers = new ArrayList<>();

    public Book(String name, String category, String author, int pageCount){
        this.bookId = name.hashCode();
        this.name = name;
        this.category = category;
        this.author = author;
        this.pageCount = pageCount;
    }

    ArrayList<User> getReaders(){
        return readers;
    }

}

BookProgress.java

package OOPDesign.onlineBookReaderSystem;

public class BookProgress  {

    long userId;
    Book book;
    int  resumedPage;

    public BookProgress(Book book, long userId) {
        this.book = book;
        this.userId = userId;
        this.resumedPage = 0;
    }

    public void setResumedPage(int resumedPage) {
        this.resumedPage = resumedPage;
    }

    public void pageForward(){
        resumedPage++;
        setResumedPage(resumedPage);
    }

    public void pageBackward(){
        resumedPage--;
        setResumedPage(resumedPage);
    }
}

FictionBook.java

package OOPDesign.onlineBookReaderSystem;

    public class FictionBook extends Book {

        public FictionBook(String name, String category, String author, int pageCount) {
            super(name,"Fiction", author, pageCount);
        }

    }

Novel.java

package OOPDesign.onlineBookReaderSystem;

public class Novel extends  Book {
    public Novel(String name, String category, String author, int pageCount) {
        super(name, "Novel", author, pageCount);
    }
}

OnlineReaderSystem.java

package OOPDesign.onlineBookReaderSystem;

public class OnlineReaderSystem {

    public static void main(String[] args) {

        // Create user
        User userNes = new User("Nesly", "Nesly","Password");

        // Create book
        Book bookFiction = new FictionBook("Fiction Book", "Fiction", "James",320);

        // User login
        userNes.login("Nesly","password");

        // Start reading book
        userNes.addBook(bookFiction);

        userNes.startReading(bookFiction);

        userNes.finishReading(bookFiction);

    }
}

User.java

package OOPDesign.onlineBookReaderSystem;

import java.time.LocalDateTime;
import java.time.format.DateTimeFormatter;
import java.util.Date;
import java.util.HashMap;
import java.util.concurrent.Semaphore;

public class User {

    private long userId;
    private String name;

    private String subcriptionType;
    private Date   subsciptionDate;

    private String login_UserId;
    private String login_Password;

    private String lastLoginDate;

    Semaphore semaphoreReader;

    HashMap<Book, BookProgress> userBooks = new HashMap<>();

    private String creditCardInfo;

    public User(String name, String login_UserId, String login_Password) {
        this.userId = name.hashCode();
        this.name = name;

        this.subcriptionType = "Classic";
        this.login_UserId = login_UserId;
        this.login_Password = login_Password;
    }


    public void login(String loginUser, String login_Password){

        if(this.login_UserId == login_UserId && this.login_Password == login_Password) {

            System.out.println("Welcome " + name);
            DateTimeFormatter dtf = DateTimeFormatter.ofPattern("yyyy/MM/dd HH:mm:ss");
            LocalDateTime now = LocalDateTime.now();
            lastLoginDate = dtf.format(now);
        }else {
            System.out.println("Unsuccessful login  " + name);
        }

    }

    public void addBook(Book book){
        userBooks.put(book, new BookProgress(book,userId));
    }

    public HashMap<Book, BookProgress> getRegisteredBooks(){
        return this.userBooks;
    }


    public int  startReading(Book book) throws InterruptedException {

        int resumedPage =  userBooks.get(book).resumedPage;
        BookProgress progress = userBooks.get(book);

        for(int i=0;i<50;i++){
          progress.pageForward();
        }

        System.out.println("Started reading");
        return resumedPage;
    }

    public void  finishReading(Book book){
        BookProgress progress = userBooks.get(book);
        System.out.println("Finished reading at "+ progress.resumedPage);
    }

}

Answer 1

Some basics...

throws

public int startReading(Book book) throws InterruptedException

This exception is never dealt with in main (it's not caught or declared). It also doesn't look like it's thrown anywhere. Don't add unnecessary throws.

unused

private Date subsciptionDate;

This is never used. Unnecessary code creates confusion by making the relevant code harder to find.

Other values are assigned, but the value is never used anywhere:

private String lastLoginDate;

casing

Generally, java variables are camelCase, your mixing styles with snake_case in the same declarations. Consistency really is key, but prefer camelCase:

public void login(String loginUser, String login_Password) {

access modifiers

Java's default access modifier, if you don't supply one, is package private. So, if you declare fields in classes without putting private in front of them, they can be modified by any other class in the same package. If this is really what you want, then fine, but otherwise, add the modifier.

long userId;
Book book;
int  resumedPage;

Where you're wanting them to be accessible to child classes, then you should be declaring them as protected, however think carefully about whether or not you want child classes to have complete access to the implementation of their parent, do you want them to be able to modify the bookId, or would an accessor make more sense?

long bookId;

final

You're setting most of a lot of your fields in the constructor and then never modifying them. If this is likely to be the end state (you're not expecting the fields to change), then you should declare the fields as final.

Constructor parameters

You don't need to have the same constructor parameters on parent classes as children. If you aren't going to use category, don't pass it into your FictionBook class, you can simply do:

public FictionBook(String name, String author, int pageCount) {
    super(name,"Fiction", author, pageCount);
}

Book model

I'm not sure it makes sense. I think of Novel's as fiction books... you're treating them as distinct types, albeit books.

String comparisons

Usually you'd use .equals, rather than == to compare strings. One does an actual string comparison, the other compares references...

if (this.login_UserId == login_UserId && this.login_Password == login_Password)

I'll stop at this point, because it actually looks like you're creating a user with 'password' and logging in as 'Password' which seems like a bug, or is unsuccessful login your expectation?

Answer 2

Naming conventions

package OOPDesign.onlineBookReaderSystem;

Java has well established naming conventions. Package names should be all lower case. This should thus be oopdesign.onlinebookreadersystem.

Single responsibility

public abstract class Book {
    ...
    ArrayList<User> readers = new ArrayList<>();

A book is a data object. It should contain information that is intrinsic to the book. A book should not be responsible for keeping track of it's readers. Especially when you seem to duplicate the book-reader connection in the BookProgress class by having it refer to the book and the user ID. To me the BookProgress class would be sufficient to establish the book-reader connection.

And since the User class is also responsible for keeping track of the books that a user reads, you have all classes referring to each other in a happy little bowl of spaghetti. :) Try drawing the relationships to a UML diagram and you'll see how it is a directed graph instead of a tree.

You should extract the book keeping into a separate ReadingProgressTracker that keeps track what book each user is reading and keep the Book and User as simple data objects. When the book keeping is extracted to it's own class it'll be much easier to add persistence to the progress tracking.

When designing a class hierarchy it helps to literally list the responsibilities of a class to see if you are following single responsibility principle. E.g. in this case "Book-class is responsible for holding book information and the reading progress of the users who are reading the book." The "and" is a keyword that should ring alarm bells. The difficult trick is to know how to correctly categorize the responsibilities (e.g. not grouping the book reading progress into the "book information" category).

Composition vs inheritance

public class FictionBook extends Book {

You don't seem to gain any advantage from inheriting book categories from the base class. All you do is hard code a string representation of the category name to the super class. And you lose the ability to add new book categories without changing the code. Thus the book should be a concrete class.

Testing

public class OnlineReaderSystem {

This is more of a trial to see if the code runs instead of a "reading system." You should replace this with a set of unit tests.

Answer 3

I have done some revisions. Thanks

Book.java (New version) Converted a concrete class. No more keep "readers" information in book class.

        package oopdesign.onlineBookReaderSystem;

        public class Book {

            private long bookId;
            private String name;
            private String category;
            private String author;
            private int pageCount;

            public Book(String name, String category, String author, int pageCount){
                this.bookId = name.hashCode();
                this.name = name;
                this.category = category;
                this.author = author;
                this.pageCount = pageCount;
            }

        }

    /* Old version
    package OOPDesign.onlineBookReaderSystem;

    import java.util.ArrayList;

    public abstract class Book {

        long bookId;
        String name;
        String category;
        String author;
        int pageCount;

        ArrayList<User> readers = new ArrayList<>();

        public Book(String name, String category, String author, int pageCount){
            this.bookId = name.hashCode();
            this.name = name;
            this.category = category;
            this.author = author;
            this.pageCount = pageCount;
        }

        ArrayList<User> getReaders(){
            return readers;
        }

    }

    */

BookProgress.java (New version) startReading & finishReading methods carried to here from User.java class.

        package oopdesign.onlineBookReaderSystem;

        public class BookProgress  {

            User user;
            Book book;
            int  resumedPage;

            public BookProgress(Book book, User user) {
                this.book = book;
                this.user = user;
                this.resumedPage = 0;
            }

            public void setResumedPage(int resumedPage) {
                this.resumedPage = resumedPage;
            }

            public int getResumedPage() { return resumedPage;  }

            public void pageForward(){
                resumedPage++;
                setResumedPage(resumedPage);
            }

            public void pageBackward(){
                 resumedPage--;
                setResumedPage(resumedPage);
            }

            public int startReading() {

                int resumedPage =  this.resumedPage;

                for(int i=0;i<50;i++){
                    pageForward();
                }

                System.out.println("Started reading");
                return resumedPage;
            }

            public void  finishReading(){
                System.out.println("Finished reading at "+ resumedPage);
            }

        }


    /* Old version

    package OOPDesign.onlineBookReaderSystem;

    public class BookProgress  {

        long userId;
        Book book;
        int  resumedPage;

        public BookProgress(Book book, long userId) {
            this.book = book;
            this.userId = userId;
            this.resumedPage = 0;
        }

        public void setResumedPage(int resumedPage) {
            this.resumedPage = resumedPage;
        }

        public void pageForward(){
            resumedPage++;
            setResumedPage(resumedPage);
        }

        public void pageBackward(){
            resumedPage--;
            setResumedPage(resumedPage);
        }
    }
    */

/* Newly added */ Library.java

    package oopdesign.onlineBookReaderSystem;

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

    public class Library {

        List<Book> library;

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

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

        public List<Book> getBookList(){
            return library;
        }

    }

OnlineReaderSystem.java (New version)

 private Library library;
 private UserManager userConsole;
 private BookProgress progress;    

connections have been added.

    package oopdesign.onlineBookReaderSystem;

    import java.util.List;

    public class OnlineReaderSystem {

        private Library library;
        private UserManager userConsole;
        private BookProgress progress;

        public OnlineReaderSystem() {
            userConsole = new UserManager();
            library = new Library();
        }

        public static void main(String[] args) {

            OnlineReaderSystem onlineReaderSystem = new OnlineReaderSystem();

            // Create user
            User userNes = new User("Nesly", "Nesly","Password");

            onlineReaderSystem.userConsole.addUser(userNes);

            List<User> userAllList = onlineReaderSystem.userConsole.getAllUsers();

            for(User u: userAllList){
                System.out.println(u.getName());
            }

            // Create book
            Book bookFiction = new Book("Fiction Book", "Fiction", "James",320);

            onlineReaderSystem.library.addBook(bookFiction);

            // User login
            userNes.login("Nesly","password");

            // Start reading book
            onlineReaderSystem.progress = new BookProgress(bookFiction, userNes);

            onlineReaderSystem.progress.startReading();

            onlineReaderSystem.progress.finishReading();

            int page = onlineReaderSystem.progress.getResumedPage();

            System.out.println(page);
        }

    }

/* Old version
package OOPDesign.onlineBookReaderSystem;

public class OnlineReaderSystem {

    public static void main(String[] args) {

        // Create user
        User userNes = new User("Nesly", "Nesly","Password");

        // Create book
        Book bookFiction = new FictionBook("Fiction Book", "Fiction", "James",320);

        // User login
        userNes.login("Nesly","password");

        // Start reading book
        userNes.addBook(bookFiction);

        userNes.startReading(bookFiction);

        userNes.finishReading(bookFiction);

    }
}

*/

User.java (New version) addBook method removed getRegisteredBooks method removed startReading method removed finishReading method removed

    package oopdesign.onlineBookReaderSystem;

    import java.time.LocalDateTime;
    import java.time.format.DateTimeFormatter;
    import java.util.Date;

    public class User {

        private long userId; 
        private String name;

        private String subcriptionType;
        private Date   subsciptionDate;

        private String loginUserId;
        private String loginPassword;
        private String lastLoginDate;

        private String creditCardInfo;

        public User(String name, String loginUserId, String loginPassword) {
            this.userId = name.hashCode();
            this.name = name;

            this.subcriptionType = "Classic";
            this.loginUserId = loginUserId;
            this.loginPassword = loginPassword;
        }


        public void login(String loginUser, String login_Password){

            if(this.loginUserId.equals(loginUserId) && this.loginPassword.equals(login_Password)) {
                System.out.println("Welcome " + name);
                DateTimeFormatter dtf = DateTimeFormatter.ofPattern("yyyy/MM/dd HH:mm:ss");
                LocalDateTime now = LocalDateTime.now();
                lastLoginDate = dtf.format(now);
            }else {
                System.out.println("Unsuccessful login  " + name);
            }

        }


        public String getName() {
            return name;
        }

        public String getSubcriptionType() {
            return subcriptionType;
        }

        public Date getSubsciptionDate() {
            return subsciptionDate;
        }

    }
/* Old version
import java.time.LocalDateTime;
import java.time.format.DateTimeFormatter;
import java.util.Date;
import java.util.HashMap;
import java.util.concurrent.Semaphore;

public class User {

    private long userId;
    private String name;

    private String subcriptionType;
    private Date   subsciptionDate;

    private String login_UserId;
    private String login_Password;

    private String lastLoginDate;

    Semaphore semaphoreReader;

    HashMap<Book, BookProgress> userBooks = new HashMap<>();

    private String creditCardInfo;

    public User(String name, String login_UserId, String login_Password) {
        this.userId = name.hashCode();
        this.name = name;

        this.subcriptionType = "Classic";
        this.login_UserId = login_UserId;
        this.login_Password = login_Password;
    }


    public void login(String loginUser, String login_Password){

        if(this.login_UserId == login_UserId && this.login_Password == login_Password) {

            System.out.println("Welcome " + name);
            DateTimeFormatter dtf = DateTimeFormatter.ofPattern("yyyy/MM/dd HH:mm:ss");
            LocalDateTime now = LocalDateTime.now();
            lastLoginDate = dtf.format(now);
        }else {
            System.out.println("Unsuccessful login  " + name);
        }

    }

    public void addBook(Book book){
        userBooks.put(book, new BookProgress(book,userId));
    }

    public HashMap<Book, BookProgress> getRegisteredBooks(){
        return this.userBooks;
    }


    public int  startReading(Book book) throws InterruptedException {

        int resumedPage =  userBooks.get(book).resumedPage;
        BookProgress progress = userBooks.get(book);

        for(int i=0;i<50;i++){
          progress.pageForward();
        }

        System.out.println("Started reading");
        return resumedPage;
    }

    public void  finishReading(Book book){
        BookProgress progress = userBooks.get(book);
        System.out.println("Finished reading at "+ progress.resumedPage);
    }

}
*/

// Newly added UserManager.java

package oopdesign.onlineBookReaderSystem;

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

public class UserManager {

    List<User> users;

    public UserManager(){
        users = new ArrayList<>();
    }

    public void addUser(User user){
        users.add(user);
    }

    public List<User> getAllUsers(){
        return users;
    }

}