file: Implement minor design changes

I had been returning success / failed boolean values for various
operations.  I think it's better to throw an exception in these
"exceptional" circumstances since error values can give more information
about why a function failed.

Signed-off-by: Anna Schumaker <schumaker.anna@gmail.com>

design.txt

@@ -72,12 +72,11 @@ Errors: (include/error.h
 
 Error Codes:
 	enum error_t {
-		EEXIST = 1,
-		ELEGACY = 2,
-		EINVAL = 3,
-		EOPEN = 4,
-		ENOTRACK = 5,
-		EAUDIO = 6,
+		EAUDIO = 1,	/* Audio error */
+		EEXIST,		/* Requested object does not exist */
+		EINVAL,		/* Invalid operation requested */
+		EIO,		/* I/O error */
+		ENOTRACK,
 	};
 
 

design/error.txt

@@ -11,10 +11,9 @@ Errors: (include/error.h
 
 Error Codes:
 	enum error_t {
-		EEXIST = 1,
-		ELEGACY = 2,
-		EINVAL = 3,
-		EOPEN = 4,
-		ENOTRACK = 5,
-		EAUDIO = 6,
+		EAUDIO = 1,	/* Audio error */
+		EEXIST,		/* Requested object does not exist */
+		EINVAL,		/* Invalid operation requested */
+		EIO,		/* I/O error */
+		ENOTRACK,
 	};

include/error.h

@@ -0,0 +1,15 @@
+/*
+ * Copyright 2013 (c) Anna Schumaker.
+ */
+#ifndef OCARINA_ERROR_H
+#define OCARINA_ERROR_H
+
+enum error_t {
+	EAUDIO = 1,	/* Audio error */
+	EEXIST,		/* Requested object does not exist */
+	EINVAL,		/* Invalid operation requested */
+	EIO,		/* I/O error */
+	ENOTRACK,
+};
+
+#endif /* OCARINA_ERROR_H */

include/file.h

@@ -30,17 +30,17 @@ private:
 	unsigned int version;
 
 	void find_dir(std::string &);
-	bool open_read();
-	bool open_write();
+	void open_read();
+	void open_write();
 
 public:
-	File(std::string, FileLocHint);
+	File(const std::string &, FileLocHint);
 	~File();
 	const char *get_filepath();
 	const unsigned int get_version();
 	bool exists();
-	bool open(OpenMode);
-	bool close();
+	void open(OpenMode);
+	void close();
 	std::string getline();
 };
 

lib/file.cpp

@@ -1,6 +1,7 @@
 /*
  * Copyright 2013 (c) Anna Schumaker.
  */
+#include <error.h>
 #include <file.h>
 #include <print.h>
 
@@ -43,7 +44,7 @@ void File :: find_dir(std::string &res)
 	}
 }
 
-File :: File(std::string path, FileLocHint file_hint)
+File :: File(const std::string &path, FileLocHint file_hint)
       : mode(NOT_OPEN), hint(file_hint)
 {
 	std::string dir;
@@ -78,75 +79,72 @@ bool File :: exists()
 	return g_file_test(filepath.c_str(), G_FILE_TEST_EXISTS);
 }
 
-bool File :: open_read()
+void File :: open_read()
 {
 	if (!exists()) {
 		dprint("ERROR: File does not exist (%s)\n", filepath.c_str());
-		return false;
+		throw -EEXIST;
 	}
 
 	std::fstream::open(filepath.c_str(), std::fstream::in);
 	if (std::fstream::fail()) {
 		dprint("ERROR: Could not open file for reading (%s)\n", filepath.c_str());
-		return false;
+		throw -EIO;
 	}
 
 	mode = OPEN_READ;
 	std::fstream::operator>>(version);
 	getline();
-	return std::fstream::good();
 }
 
-bool File :: open_write()
+void File :: open_write()
 {
 	std::string dir;
 	if (hint == FILE_TYPE_LEGACY) {
 		dprint("ERROR: Cannot write to legacy files (%s)\n", filepath.c_str());
-		return false;
+		throw -EIO;
 	}
 
 	find_dir(dir);
 	if (g_mkdir_with_parents(dir.c_str(), 0755) != 0) {
 		dprint("ERROR: Could not make directory (%s)\n", dir.c_str());
-		return false;
+		throw -EIO;
 	}
 
 	std::fstream::open(filepath.c_str(), std::fstream::out);
 	if (std::fstream::fail()) {
 		dprint("ERROR: Could not open file for writing (%s)\n", filepath.c_str());
-		return false;
+		throw -EIO;
 	}
 
 	mode = OPEN_WRITE;
 	std::fstream::operator<<(FILE_VERSION) << std::endl;
-	return std::fstream::good();
 }
 
-bool File :: open(OpenMode m)
+void File :: open(OpenMode m)
 {
 	if (mode != NOT_OPEN) {
 		dprint("ERROR: File is already open (%s)\n", filepath.c_str());
-		return false;
+		throw -EIO;
 	}
 
 	if (m == NOT_OPEN) {
 		dprint("ERROR: NOT_OPEN is not a legal OpenMode (%s)\n", filepath.c_str());
-		return false;
+		throw -EINVAL;
 	} else if (hint == FILE_TYPE_INVALID) {
 		dprint("ERROR: A file with hint = FILE_TYPE_INVALID cannot be opened\n");
-		return false;
+		throw -EINVAL;
 	} else if (m == OPEN_READ)
-		return open_read();
+		open_read();
 	else /* m == OPEN_WRITE */
-		return open_write();
+		open_write();
 }
 
-bool File :: close()
+void File :: close()
 {
 	if (mode != NOT_OPEN)
 		std::fstream::close();
 	mode = NOT_OPEN;
-	return std::fstream::good();
 }
 
 std::string File :: getline()

tests/file/file.cpp

@@ -1,6 +1,7 @@
 /*
  * Copyright 2013 (c) Anna Schumaker.
  */
+#include <error.h>
 #include <file.h>
 #include <print.h>
 
@@ -50,8 +51,14 @@ void test_a(int testno, bool valid, FileLocHint hint)
  */
 void test_b(int testno, bool valid, FileLocHint hint)
 {
+	int status = 0;
 	File *f = get_file(testno, 'b', valid, hint);
-	test_result(f->open(NOT_OPEN) == false, true);
+	try {
+		f->open(NOT_OPEN);
+	} catch (int error) {
+		status = error;
+	}
+	test_result(status == -EINVAL, true);
 }
 
 /*
@@ -59,8 +66,17 @@ void test_b(int testno, bool valid, FileLocHint hint)
  */
 void test_c(int testno, bool valid, FileLocHint hint)
 {
+	int status = 0;
 	File *f = get_file(testno, 'c', valid, hint);
-	test_result(f->open(OPEN_READ) == false, true);
+	try {
+		f->open(OPEN_READ);
+	} catch (int error) {
+		status = error;
+	}
+	if (valid == false)
+		test_result(status == -EINVAL, true);
+	else
+		test_result(status == -EEXIST, true);
 }
 
 /*
@@ -68,12 +84,21 @@ void test_c(int testno, bool valid, FileLocHint hint)
  */
 void test_d(int testno, bool valid, FileLocHint hint)
 {
+	int status = 0;
 	File *f = get_file(testno, 'd', valid, hint);
-	bool ret = f->open(OPEN_WRITE);
 
-	if (inval_or_legacy(valid, hint))
-		ret = !ret;
-	test_result(ret, inval_or_legacy(valid, hint));
+	try {
+		f->open(OPEN_WRITE);
+	} catch (int error) {
+		status = error;
+	}
+
+	if (valid == false)
+		test_result(status == -EINVAL, true);
+	else if (hint == FILE_TYPE_LEGACY)
+		test_result(status == -EIO, true);
+	else
+		test_result(status == 0, false);
 }
 
 /*
@@ -81,8 +106,14 @@ void test_d(int testno, bool valid, FileLocHint hint)
  */
 void test_e(int testno, bool valid, FileLocHint hint)
 {
+	int status = 0;
 	File *f = get_file(testno, 'e', valid, hint);
-	test_result(f->close(), false);
+	try {
+		f->close();
+	} catch (int error) {
+		status = error;
+	}
+	test_result(status == 0, false);
 }
 
 /*
@@ -91,14 +122,28 @@ void test_e(int testno, bool valid, FileLocHint hint)
 void test_f(int testno, bool valid, FileLocHint hint)
 {
 	File *f;
+	int status = 0;
 
 	if (inval_or_legacy(valid, hint))
 		return;
 
 	f = get_file(testno, 'f', valid, hint);
-	test_result(f->open(OPEN_WRITE), false);
+
+	try {
+		f->open(OPEN_WRITE);
+	} catch (int error) {
+		status = error;
+	}
+	test_result(status == 0, false);
 	print("         ");
-	test_result(f->open(OPEN_READ) == false, true);
+
+	status = 0;
+	try {
+		f->open(OPEN_READ);
+	} catch (int error) {
+		status = error;
+	}
+	test_result(status == -EIO, true);
 }
 
 /*
@@ -107,18 +152,38 @@ void test_f(int testno, bool valid, FileLocHint hint)
 void test_g(int testno, bool valid, FileLocHint hint)
 {
 	File *f;
+	int status = 0;
 
 	if (inval_or_legacy(valid, hint))
 		return;
 
 	f = get_file(testno, 'g', valid, hint);
-	if (!test_result(f->open(OPEN_WRITE), false))
+
+	try {
+		f->open(OPEN_WRITE);
+	} catch (int error) {
+		status = error;
+	}
+	if (!test_result(status == 0, false))
 		return;
-	if (!test_result(f->close(), true))
+
+	try {
+		f->close();
+	} catch (int error) {
+		status = error;
+	}
+	if (!test_result(status == 0, true))
 		return;
-	if (!test_result(f->open(OPEN_READ), true))
+
+	try {
+		f->open(OPEN_READ);
+	} catch (int error) {
+		status = error;
+	}
+	if (!test_result(status == 0, true))
 		return;
-	test_result(f->close(), true);
+
+	f->close();
 }
 
 /*
@@ -127,7 +192,7 @@ void test_g(int testno, bool valid, FileLocHint hint)
 void test_h(int testno, bool valid, FileLocHint hint)
 {
 	File *f;
-	int a, b, c, d, e;
+	int a, b, c, d, e, status = 0;
 
 	if (inval_or_legacy(valid, hint))
 		return;
@@ -136,20 +201,35 @@ void test_h(int testno, bool valid, FileLocHint hint)
 	/*
 	 * Write data to file
 	 */
-	if (!test_result(f->open(OPEN_WRITE), false))
+	try {
+		f->open(OPEN_WRITE);
+	} catch (int error) {
+		status = error;
+	}
+	if (!test_result(status == 0, false))
 		return;
 
 	(*f) << "Hello, World!" << std::endl;
 	(*f) << "This is a multi-line file =)" << std::endl;
 	(*f) << "1 2 3 4 5" << std::endl;
 
-	if (!test_result(f->close(), true))
+	try {
+		f->close();
+	} catch (int error) {
+		status = error;
+	}
+	if (!test_result(status == 0, true))
 		return;
 
 	/*
 	 * Read data back from disk
 	 */
-	if (!test_result(f->open(OPEN_READ), true))
+	try {
+		f->open(OPEN_READ);
+	} catch (int error) {
+		status = error;
+	}
+	if (!test_result(status == 0, true))
 		return;
 
 	print("         File version is: %u\n", f->get_version());
@@ -168,7 +248,12 @@ void test_h(int testno, bool valid, FileLocHint hint)
 	/*
 	 * Close file
 	 */
-	test_result(f->close(), true);
+	try {
+		f->close();
+	} catch (int error) {
+		status = error;
+	}
+	test_result(status == 0, true);
 }
 
 static void(*tests[])(int, bool, FileLocHint) = {

tests/file/file.good

@@ -11,7 +11,6 @@ Test 1f: Passed
 Test 1g: Passed
          Passed
          Passed
-         Passed
 Test 1h: Passed
          Passed
          Passed
@@ -46,7 +45,6 @@ Test 3f: Passed
 Test 3g: Passed
          Passed
          Passed
-         Passed
 Test 3h: Passed
          Passed
          Passed