I'm going to repeat some of what others said and add a few things which are very important to learn and no one mentioned them (for known reasons).
Naming Conventions
You should use the naming conventions defined in the Java Language Specifications, JLS 6.1. Declarations (starting from the small text "Naming Conventions"). The JLS is something you will want to familiarize with over time, although it's fairly technical, so don't worry much about it right now.
You will want to rename to lowercase capacity
and volume
.
Use Declared Values
If you intended to use the number held in capacity
, use the field reference instead. Then you only need to change the number in one place and it's also clearer where the numbers in the code come from (magic numbers).
You're using it in the array initialization (which you do twice) and in the print statement.
Constants vs. Variables
Determine which of the values you declare are constant and which can change. Those which can't change should be declared final
. Consider setting these constants in the constructor if you want to allow a different value for each instance.
Is capacity
something that is constant for each Library
instance? Is title
constant for each Book
? If so, declare them final
. Maybe request the capacity
value in the constructor like you did for title
.
Encapsulate
Use visibility modifiers to control access to an object's fields and methods. This allows each object to control its behavior and expose its services. Search online for the benefits of encapsulation, like here.
Looks like all the fields should be private
. If you want someone from outside the class to be able to read and/or write them, use getters and/or setters.
Override Object
's Methods
This is very important! In fact, Brian Goetz (Java architect) talked about this recently, saying how developers don't do this, why, and why they should.
You did well overriding Book
's toString
. You will really want to override its equals
and hashCode
as well. Your IDE has tools to generate these methods for you. You can also look at Project Lombok that help to reduce boilerplate code.
It is extremely recommended that you use the @Override
annotation when overriding methods.
Usage of this
Generally you don't want to use this
whenever possible, but to you use it as least as possible. Opinions vary on this one (no pun intended).
return this.title;
could be return title;
, which is more readable. Same for Library
usages.
Flow Control
When using if
and else
, check the logic to see what is the best way to specify your flow.
You are checking if (volume < capacity)
and then else if (volume >= capacity)
, but if the first check fails, then the second must succeed, so no need for it - a simple else
will do.
"foreach" Loop
When iterating over elements where the index is not important, you should use the "foreach" (or "enhanced for") loop. Search online for the full comparison, like here.
for (int i = 0; i < volume; i++)
could be for (Book book : storage)
.
Beware the equals
NPE Trap
The equals
method my IDE generated checks for null
s, but familiarize yourself with the Objects
class and its methods, in this case the static equals
methods, which checks for null
s for you. Note that o1.equals(o2)
will throw a NPE if o1 == null
, but not if o2 == null
, which is where the above method comes in handy.
If someone passes a null
String to your search
method, you will get a NPE.
Consider Appropriate Return Types
What return type suits an operation the most?
Your add
method could possibly return a boolean
to specify if the book was added or not. Java collections do this as well (see below).
Consider Using Optional<T>
... but don't overdo it. This was a topic for another talk by the Java architects, this time by Stuart Marks. I won't go into all of this, but beware when returning null
. What you have currently is perfectly acceptable.
Your search
can return and Optional<Book>
, but I wouldn't bother with it. Document your method and say that it returns null
if no match is found.
Consider Using Collections
Java's collection offer a lot of benefits over arrays. Again, huge topic, search online.
If you can use a List<Book>
instead of your array, you could benefit from not needing the volume
field at all - instead you have list.size()
. This allows to simplify your add
method. List
also returns a boolean
for its add
method in case you want to use that as a return value.
Consider Using Streams
Java 8's streams allow to perform bulk operation easily and they are more readable. Search online for advantages of streams over loops.
Your for
/"foreach" loop could be replaced with a stream that does the search and finds results in one go. See code below.
Nitpick
You don't need a ;
at the end of a method declaration.
The toString
of Book
has that.
Code time!
Book
:
public class Book {
private final String title;
public Book(String title) {
this.title = title;
System.out.println("Book " + title + " created.");
}
public String getTitle() {
return title;
}
@Override
public int hashCode() {
final int prime = 31;
int result = 1;
result = prime * result + ((title == null) ? 0 : title.hashCode());
return result;
}
@Override
public boolean equals(Object obj) {
if (this == obj)
return true;
if (obj == null)
return false;
if (getClass() != obj.getClass())
return false;
Book other = (Book) obj;
if (title == null) {
if (other.title != null)
return false;
} else if (!title.equals(other.title))
return false;
return true;
}
@Override
public String toString() {
return this.title;
}
}
Library
with arrays (you can play with List
yourself):
public class Library {
private final int capacity;
private int volume = 0;
private Book[] storage;
public Library(int capacity) {
this.capacity = capacity;
storage = new Book[capacity];
System.out.println("Hello, I am a library, which can store up to " + capacity + " books!");
}
public void add(Book book) {
if (volume < capacity) {
storage[volume] = book;
System.out.println("I added the book " + book + "!");
volume++;
} else
System.out.println("The library is full!");
}
public Book search(String title) {
for (Book book : storage) {
if (Objects.equals(title, book.getTitle())) {
System.out.println("The book with the title " + title + " exists in the library!");
return book;
}
}
System.out.println("The book with the title " + title + " does not exist in the library!");
return null;
//or with streams: returns an Optional<Book>
return Arrays.stream(storage)
.filter(book -> Objects.equals(title, book.getTitle()))
.findAny();
}
}
Volume
can be quite misleading, since books can be numbered by "volume". A better name would becurrentAmountOfBooks
. The current prefix is imo very important, since it tells a lot about the variable. \$\endgroup\$