Media lending library

My assignment was to make a "Lending Library" that keeps track of items. We were advised to create two classes and a main method.

What I am unsure of is if any of my code would be considered bad practice or unnecessary, especially when it comes to declaring objects and instantiating classes.

Also, our professor suggested we use getter/setter methods for all class fields. Would those be necessary for this program?

```
class MediaItem { //fields
String title;
String format;
boolean onLoan;
String loanedTo;
String dateLoaned;

MediaItem(){ //default constructor
title = null;
format = null;
onLoan = false;
loanedTo = null;
dateLoaned = null;
}
MediaItem(String title, String format){ //constructor
onLoan = false;
this.title = title;
this.format = format;
}

void markOnLoan(String name, String date){ //methods
if(onLoan == true)
System.out.println(this.title + " is already loaned out");
else {
onLoan = true;
loanedTo = name;
dateLoaned = date;
}
}
void markReturned(){
if(onLoan == false)
System.out.println(this.title + " is not currently loaned out");
onLoan = false;
}
}
class Library{
static Scanner in = new Scanner(System.in); //instantiation
MediaItem t = new MediaItem();
MediaItem[] items = new MediaItem[100];
String[] str = new String[100];
int numberOfItems = 0; //fields
int check = 0;
int called = 0;

int displayMenu(){ //methods
System.out.println("1. Add new item \n2. Mark an item as on loan \n3. List all items \n4. Mark an item as returned \n5. Quit");
System.out.print("What would you like to do? ");
int a = in.

Constructors

You have a 'default' constructor and a 'real' (?) constructor that mandates initial title and format values. The question to ask here is whether any MediaItem object can carry null values. If not, then your default constructor actually becomes risky in the sense that you can carelessly create such empty MediaItem objects, but parts of your code simply assumed they shouldn't exist and... causing NullPointerExceptions when they access the null fields.

On a related note, even if you want to keep the default constructor around, you should chain/cascade your constructors properly so that it is more extensible to future changes... For example:

MediaItem() {
    this(null, null, false, null, null);
}

MediaItem(String title, String format) {
    this(title, format, false, null, null);
}

MediaItem(String title, String format, boolean onLoan, String loanedTo, String dateLoaned) {
    this.title = title;
    this.format = format;
    this.onLoan = onLoan;
    this.loanedTo = loanedTo;
    this.dateLoaned = dateLoaned;
}

This gives you the option to either:

• fully specify all five fields when instantiating an object,

• just title and format, or

• simply specify nothing at all with the 'default' constructor.

If you choose to have another three-item constructor or change what the default values should be (e.g. an empty String - ""?), it's relatively easy to do that too. More importantly, if you decide to make MediaItem an immutable class, the chained constructor approach ensures that fields are properly initialized. For more information, see my previous answer to another question here.

Logging and prompting

You are doing the following in both your markOnLoan() and markReturned() methods:

if (onLoan) {
    System.out.println("some message here");
}

First, you don't need onLoan == true/false, you can simply put if (onLoan) { ... } or if (!onLoan) { ... }.

Second, such messages to System.out is not usually recommended within your model classes as it ties them to a specific output, making it harder to change in the future. Usually, you can use a logging framework such as SLF4J to handle the logging of these messages for you. The benefits are:

• The flexibility of configuring the logging levels (e.g. you may want to show all DEBUG messages during development, but only show INFO messages for the assignment submission).

• The output's destination (e.g. to System.out or to a file).

• The output formatting (e.g. do you want prefixed timestamps? Show the class names?).

Alternatively, you may want to consider handling these cases at the callers' side. Instead of making these methods have void return types, they can return a boolean indicating if the change is successful. For example:

public boolean markOnLoan(String name, String date) {
        // ...
    }

// caller side
boolean isMarked = item.markOnLoan(name, date);
if (!isMarked) {
    System.out.println(item.title + " is already loaned out");
}

Overriding methods

Within your main() method, you have the following two snippets:

for (int b = 0; b < numberOfItems; b++) {
    if (title.equals(items[b].title)) {
        // ...
    }
}

This particular comparison should be done in your MediaItem class, by overriding its equals() method. For example:

@Override
public boolean equals(Object o) {
    return o instanceof MediaItem && title.equals(((MediaItem)o).title);
}

Often, overriding equals() should require you to override hashCode(), and you can refer to Object.equals() for more information.

if (items[c].onLoan)
    str[c] = "\n" + items[c].title + " " + items[c].format + 
                " loaned to " + items[c].loanedTo + " on " + items[c].dateLoaned;
else
    str[c] = "\n" + items[c].title + " " + items[c].format;
System.out.println(str[c] + "\n");

Constructing a String representation can be done using the toString() method in your MediaItem class as well, for example:

@Override
public String toString() {
    // not sure why you prefer a trailing newline, including as such
    return title + " " + format + 
            (onLoan ? " loaned to " + loanedTo + " on " + dateLoaned : "") + "\n";
}

When you have overridden toString(), you can simply loop through your objects and call that instead:

// using the enhanced for-loop
for (MediaItem item : items) {
    System.out.println(item);
}

Arrays vs Collections classes

It seems slightly odd that you are hard-coding your items array to be exactly 100 items, is this an assignment requirement? Generally, you can use a List implementation such as ArrayList so that you do not have to be explicitly concerned about what is a suitable maximum number of items to store.

Variable names

Just a minor point, but I could not understand why you will call your Library object as track:

Library track = new Library();

StackExchange Code Review Q#95049, answer score: 4