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] Zhang Ying #509

Open
wants to merge 2 commits 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
11 changes: 6 additions & 5 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){
setStorage(storageFile);
Logic(Storage storage, AddressBook addressBook){
setStorage(storage);
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
9 changes: 9 additions & 0 deletions src/seedu/addressbook/storage/Storage.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
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.

public abstract AddressBook load() throws StorageOperationException, seedu.addressbook.storage.StorageStub.StorageOperationException;
public abstract String getPath();

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 void save(AddressBook addressBook) throws StorageOperationException;
}
2 changes: 1 addition & 1 deletion src/seedu/addressbook/storage/StorageFile.java
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
/**
* 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";
Expand Down
128 changes: 128 additions & 0 deletions src/seedu/addressbook/storage/StorageStub.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,128 @@
package seedu.addressbook.storage;

import seedu.addressbook.data.AddressBook;
import seedu.addressbook.data.exception.IllegalValueException;
import seedu.addressbook.storage.jaxb.AdaptedAddressBook;

import javax.xml.bind.JAXBContext;
import javax.xml.bind.JAXBException;
import javax.xml.bind.Unmarshaller;
import java.io.*;
import java.nio.file.Path;
import java.nio.file.Paths;

/**
* Represents the file used to store address book data.
*/
public class StorageStub extends Storage {

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

/* Note: Note the use of nested classes below.
* More info https://docs.oracle.com/javase/tutorial/java/javaOO/nested.html
*/

/**
* Signals that the given file path does not fulfill the storage filepath constraints.
*/
public static class InvalidStorageFilePathException extends IllegalValueException {
public InvalidStorageFilePathException(String message) {
super(message);
}
}

/**
* Signals that some error has occured while trying to convert and read/write data between the application
* and the storage file.
*/
public static class StorageOperationException extends Exception {
public StorageOperationException(String message) {
super(message);
}
}

private final JAXBContext jaxbContext;

public final Path path;

/**
* @throws InvalidStorageFilePathException if the default path is invalid
*/
public StorageStub() throws InvalidStorageFilePathException {
this(DEFAULT_STORAGE_FILEPATH);
}

/**
* @throws InvalidStorageFilePathException if the given file path is invalid
*/
public StorageStub(String filePath) throws InvalidStorageFilePathException {
try {
jaxbContext = JAXBContext.newInstance(AdaptedAddressBook.class);
} catch (JAXBException jaxbe) {
throw new RuntimeException("jaxb initialisation error");
}

path = Paths.get(filePath);
if (!isValidPath(path)) {
throw new InvalidStorageFilePathException("Storage file should end with '.txt'");
}
}

/**
* Returns true if the given path is acceptable as a storage file.
* The file path is considered acceptable if it ends with '.txt'
*/
private static boolean isValidPath(Path filePath) {
return filePath.toString().endsWith(".txt");
}

/**
* Saves all data to this storage file.
*/
public void save(AddressBook addressBook) {

}

/**
* Loads data from this storage file.
* @throws StorageOperationException
*/
public AddressBook load() throws StorageOperationException {

Choose a reason for hiding this comment

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

Stubs don't usually do any work. So preferably, it should be empty.

try (final Reader fileReader =
new BufferedReader(new FileReader(path.toFile()))) {

final Unmarshaller unmarshaller = jaxbContext.createUnmarshaller();
final AdaptedAddressBook loaded = (AdaptedAddressBook) unmarshaller.unmarshal(fileReader);
// manual check for missing elements
if (loaded.isAnyRequiredFieldMissing()) {
throw new StorageOperationException("File data missing some elements");
}
return loaded.toModelType();

/* Note: Here, we are using an exception to create the file if it is missing. However, we should minimize
* using exceptions to facilitate normal paths of execution. If we consider the missing file as a 'normal'
* situation (i.e. not truly exceptional) we should not use an exception to handle it.
*/

// create empty file if not found
} catch (FileNotFoundException fnfe) {
final AddressBook empty = new AddressBook();
save(empty);
return empty;

// other errors
} catch (IOException ioe) {
throw new StorageOperationException("Error writing to file: " + path);
} catch (JAXBException jaxbe) {
throw new StorageOperationException("Error parsing file data format");
} catch (IllegalValueException ive) {
throw new StorageOperationException("File contains illegal data values; data type constraints not met");
}
}

public String getPath() {
return path.toString();
}

}
5 changes: 3 additions & 2 deletions test/java/seedu/addressbook/logic/LogicTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import seedu.addressbook.data.tag.Tag;
import seedu.addressbook.data.tag.UniqueTagList;
import seedu.addressbook.storage.StorageFile;
import seedu.addressbook.storage.StorageStub;

import java.util.*;

Expand All @@ -28,13 +29,13 @@ public class LogicTest {
@Rule
public TemporaryFolder saveFolder = new TemporaryFolder();

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

Choose a reason for hiding this comment

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

I notice your test case failed and i think that it is because you compare the file content... You shouldn't because stub doesnt save :P

@Before
public void setup() throws Exception {
saveFile = new StorageFile(saveFolder.newFile("testSaveFile.txt").getPath());
saveFile = new StorageStub(saveFolder.newFile("testSaveFile.txt").getPath());
addressBook = new AddressBook();
saveFile.save(addressBook);
logic = new Logic(saveFile, addressBook);
Expand Down