TransWikia.com

Online Reader System Object Oriented System

Code Review Asked by Neslihan Bozer on December 22, 2021

I have designed and Online Book Reader System as considering the previous reviews on this platform. You can find the classes below. I would be appreciated for the valuable reviews.

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

Book.java

package oopdesign.onlinebookreader;

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;
    }

}

BookProgress.java

package oopdesign.onlinebookreader;

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);
    }

}

Library.java

package oopdesign.onlinebookreader;

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

package oopdesign.onlinebookreader;

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);
    }

}

User.java

package oopdesign.onlinebookreader;

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

public class User {

    private long userId;

    public String getName() {
        return name;
    }

    public String getSubcriptionType() {
        return subcriptionType;
    }

    public Date getSubsciptionDate() {
        return subsciptionDate;
    }

    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);
        }

    }

}

UserManager.java

package oopdesign.onlinebookreader;

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;
    }

}

2 Answers

@Martin Frank has given you a few things to think about, so I'm only going to add a couple of points. Firstly, use consistent formatting it makes a difference for readability. Most IDE's will do this for you automatically. As it stands, you have some method signatures that have spaces between ) { and some that don't ){.

Secondly, your Book class has a bunch of private fields, that don't have getters, or any methods that operate on them other than the constructor that set's them. Consider not adding fields until they're actually needed, it'll save you time in the long run.

Finally, consider these methods:

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

Your pageForward increments the resumedPage field then calls the properties setter to set the field to the value that the field already has. You don't need to call setResumedPage in this scenario, since resumedPage already has the same value...

Answered by forsvarir on December 22, 2021

bloater pt 1

on my first glance i saw that two classes are not required - the name for that anti-pattern is called Middle Man:

  • Library
  • UserManager

you can easily refer directly to these List<?> (Note: if you provide more function to these classes, maybe like a filter, then these classes would make totally sense - it also would have a name then: first-class collection. Since it is not (yet) needed it also violates YAGNI

bloater pt 2

the User class is bloated - it has too much responsibility

extract these responsibilites into the proper classes

//TODO class Subscription
private String subcriptionType;
private Date   subsciptionDate;
...
public String getSubcriptionType() 
public Date getSubsciptionDate()
...
//TODO class LogIn
private String loginUserId;
private String loginPassword;
private String lastLoginDate;
...
public void login(String loginUser, String login_Password)
...

some other minor issues

  • magic numbers for(int i=0;i<50;i++)... - why 50?
  • primitve Obsession (why not use an enum for subcriptionType - that would make turn "Classic" into it's proper value)
  • primitve Obsession - String creditCardInfo - a class would make sense here (especially because you have to treat these information with special care)
  • hardcoded Strings - "Started reading", "Finished reading at "
  • use a test class instead of a main() test

Answered by Martin Frank on December 22, 2021

Add your own answers!

Ask a Question

Get help from others!

© 2024 TransWikia.com. All rights reserved. Sites we Love: PCI Database, UKBizDB, Menu Kuliner, Sharing RPP