Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[T6A1][F14-B2] walter tay #502

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions src/seedu/addressbook/logic/Logic.java
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import seedu.addressbook.data.AddressBook;
import seedu.addressbook.data.person.ReadOnlyPerson;
import seedu.addressbook.parser.Parser;
import seedu.addressbook.storage.Storage;
import seedu.addressbook.storage.StorageFile;

import java.util.Collections;
Expand All @@ -17,7 +18,7 @@
public class Logic {


private StorageFile storage;
private Storage storage;
private AddressBook addressBook;

/** The list of person shown to the user most recently. */
Expand All @@ -28,12 +29,12 @@ public Logic() throws Exception{
setAddressBook(storage.load());
}

Logic(StorageFile storageFile, AddressBook addressBook){
Logic(Storage storageFile, AddressBook addressBook){
setStorage(storageFile);
setAddressBook(addressBook);
}

void setStorage(StorageFile storage){
void setStorage(Storage storage){
this.storage = storage;
}

Expand All @@ -45,7 +46,7 @@ void setAddressBook(AddressBook addressBook){
* Creates the StorageFile object based on the user specified path (if any) or the default storage path.
* @throws StorageFile.InvalidStorageFilePathException if the target file path is incorrect.
*/
private StorageFile initializeStorage() throws StorageFile.InvalidStorageFilePathException {
private Storage initializeStorage() throws StorageFile.InvalidStorageFilePathException {
return new StorageFile();
}

Expand Down
19 changes: 19 additions & 0 deletions src/seedu/addressbook/storage/Storage.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
package seedu.addressbook.storage;

import seedu.addressbook.data.AddressBook;
import seedu.addressbook.storage.StorageFile.StorageOperationException;

public abstract class Storage {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This class and its methods should have a header comments. If not, developers who needs to inherit from this class will not know how exactly they should override the methods and other developers who write client code for this class will not know how to use it either.


/** Default file path used if the user doesn't provide the file name. */
static String DEFAULT_STORAGE_FILEPATH = "addressbook.txt";


public abstract void save(AddressBook addressBook) throws StorageOperationException;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is better to give the method header comments in the parent class rather than the child class. The child class can inherit the comment from the parent.



public abstract AddressBook load() throws StorageOperationException;

public abstract String getPath();

}
33 changes: 17 additions & 16 deletions src/seedu/addressbook/storage/StorageFile.java
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,9 @@
/**
* Represents the file used to store address book data.
*/
public class StorageFile {
public class StorageFile extends Storage {

/** Default file path used if the user doesn't provide the file name. */
public static final String DEFAULT_STORAGE_FILEPATH = "addressbook.txt";
static String DEFAULT_STORAGE_FILEPATH = "addressbook.txt";

/* Note: Note the use of nested classes below.
* More info https://docs.oracle.com/javase/tutorial/java/javaOO/nested.html
Expand Down Expand Up @@ -78,12 +77,11 @@ private static boolean isValidPath(Path filePath) {
return filePath.toString().endsWith(".txt");
}

/**
* Saves all data to this storage file.
*
* @throws StorageOperationException if there were errors converting and/or storing data to file.
*/
public void save(AddressBook addressBook) throws StorageOperationException {
/* (non-Javadoc)
* @see seedu.addressbook.storage.Storage#save(seedu.addressbook.data.AddressBook)
*/
@Override
public void save(AddressBook addressBook) throws StorageOperationException {

/* Note: Note the 'try with resource' statement below.
* More info: https://docs.oracle.com/javase/tutorial/essential/exceptions/tryResourceClose.html
Expand All @@ -103,12 +101,11 @@ public void save(AddressBook addressBook) throws StorageOperationException {
}
}

/**
* Loads data from this storage file.
*
* @throws StorageOperationException if there were errors reading and/or converting data from file.
*/
public AddressBook load() throws StorageOperationException {
/* (non-Javadoc)
* @see seedu.addressbook.storage.Storage#load()
*/
@Override
public AddressBook load() throws StorageOperationException {
try (final Reader fileReader =
new BufferedReader(new FileReader(path.toFile()))) {

Expand Down Expand Up @@ -141,7 +138,11 @@ public AddressBook load() throws StorageOperationException {
}
}

public String getPath() {
/* (non-Javadoc)
* @see seedu.addressbook.storage.Storage#getPath()
*/
@Override
public String getPath() {
return path.toString();
}

Expand Down
3 changes: 2 additions & 1 deletion test/java/seedu/addressbook/logic/LogicTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import seedu.addressbook.data.person.*;
import seedu.addressbook.data.tag.Tag;
import seedu.addressbook.data.tag.UniqueTagList;
import seedu.addressbook.storage.Storage;
import seedu.addressbook.storage.StorageFile;

import java.util.*;
Expand All @@ -28,7 +29,7 @@ public class LogicTest {
@Rule
public TemporaryFolder saveFolder = new TemporaryFolder();

private StorageFile saveFile;
private Storage saveFile;
private AddressBook addressBook;
private Logic logic;

Expand Down
12 changes: 6 additions & 6 deletions test/java/seedu/addressbook/storage/StorageFileTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ public void constructor_noTxtExtension_exceptionThrown() throws Exception {
@Test
public void load_invalidFormat_exceptionThrown() throws Exception {
// The file contains valid xml data, but does not match the AddressBook class
StorageFile storage = getStorage("InvalidData.txt");
Storage storage = getStorage("InvalidData.txt");
thrown.expect(StorageOperationException.class);
storage.load();
}
Expand All @@ -61,15 +61,15 @@ public void load_validFormat() throws Exception {

@Test
public void save_nullAddressBook_exceptionThrown() throws Exception {
StorageFile storage = getTempStorage();
Storage storage = getTempStorage();
thrown.expect(NullPointerException.class);
storage.save(null);
}

@Test
public void save_validAddressBook() throws Exception {
AddressBook ab = getTestAddressBook();
StorageFile storage = getTempStorage();
Storage storage = getTempStorage();
storage.save(ab);

assertStorageFilesEqual(storage, getStorage("ValidData.txt"));
Expand All @@ -80,15 +80,15 @@ public void save_validAddressBook() throws Exception {
/**
* Asserts that the contents of two storage files are the same.
*/
private void assertStorageFilesEqual(StorageFile sf1, StorageFile sf2) throws Exception {
private void assertStorageFilesEqual(Storage sf1, Storage sf2) throws Exception {
assertTextFilesEqual(Paths.get(sf1.getPath()), Paths.get(sf2.getPath()));
}

private StorageFile getStorage(String fileName) throws Exception {
private Storage getStorage(String fileName) throws Exception {
return new StorageFile(TEST_DATA_FOLDER + "/" + fileName);
}

private StorageFile getTempStorage() throws Exception {
private Storage getTempStorage() throws Exception {
return new StorageFile(testFolder.getRoot().getPath() + "/" + "temp.txt");
}

Expand Down