diff --git a/include/tapelib/filetape.h b/include/tapelib/filetape.h index 27e1a9f..b8ba77e 100644 --- a/include/tapelib/filetape.h +++ b/include/tapelib/filetape.h @@ -52,10 +52,22 @@ const static int FT_SEEK_OFFSET = FT_CELL_SIZE + 1; */ class FileTape : public Tape { private: + std::string file_name; std::fstream file; FileTapeSettings settings; + bool temp = false; public: + /** + * Creating copies of FileTape creates 2+ fstreams to one file, which + * could introduce IO errors or data races if implemented incorrectly - + * mark the copy constructor as deleted. Moving is fine. + */ + FileTape(const FileTape &) = delete; + FileTape(FileTape &&) noexcept = default; + FileTape &operator=(const FileTape &) = delete; + FileTape &operator=(FileTape &&tape) = default; + /** * Initializes a new instance of FileTape that will open an existing file * with the specified name, already filled with data. File must be read and @@ -64,8 +76,8 @@ class FileTape : public Tape { * @param file_name Name of a file to be opened. * @param settings FileTape settings */ - explicit FileTape(const std::string &file_name, - FileTapeSettings settings = FT_DEFAULT_SETTINGS); + FileTape(std::string file_name, + FileTapeSettings settings = FT_DEFAULT_SETTINGS); /** * Initializes a new instance of FileTape that will create and open a file @@ -76,8 +88,8 @@ class FileTape : public Tape { * @param file_name Name of a file to be created and opened * @param settings FileTape settings */ - explicit FileTape(size_t size, const std::string &file_name, - FileTapeSettings settings = FT_DEFAULT_SETTINGS); + FileTape(size_t size, std::string file_name, + FileTapeSettings settings = FT_DEFAULT_SETTINGS); /** * Initializes a new instance of FileTape that will create and open a @@ -86,8 +98,7 @@ class FileTape : public Tape { * @param size Size of a newly created FileTape * @param settings FileTape settings */ - explicit FileTape(size_t size, - FileTapeSettings settings = FT_DEFAULT_SETTINGS); + FileTape(size_t size, FileTapeSettings settings = FT_DEFAULT_SETTINGS); /** * Advances the underlying fstream to a new line. @@ -116,6 +127,18 @@ class FileTape : public Tape { * @param data Number, that should be written to the current line. */ void write(uint32_t data) override; + + ~FileTape() override; + + // Define getters in header file since they are implicitly inlined + + bool is_temp() const { return this->temp; } + + const FileTapeSettings &get_settings() const { return this->settings; } + + const std::string &get_file_name() const { return this->file_name; } + + const std::fstream &get_file() const { return this->file; } }; } // namespace tape diff --git a/include/tapelib/tape.h b/include/tapelib/tape.h index 5c7667a..b774043 100644 --- a/include/tapelib/tape.h +++ b/include/tapelib/tape.h @@ -17,11 +17,16 @@ namespace tape { */ class Tape { public: + /** + * Compiler provided constructors and destructor definitions. + * Make move constructor and destructor default, copy construcotr delete, + * and allow destrucor to be overriden. + */ Tape() = default; - Tape(const Tape &) = default; - Tape(Tape &&) = delete; - Tape &operator=(const Tape &) = default; - Tape &operator=(Tape &&) = delete; + Tape(const Tape &) = delete; + Tape(Tape &&) = default; + Tape &operator=(const Tape &) = delete; + Tape &operator=(Tape &&) = default; virtual ~Tape() = default; /** diff --git a/include/tapelib/tape_util.h b/include/tapelib/tape_util.h index 1433961..2b6e74e 100644 --- a/include/tapelib/tape_util.h +++ b/include/tapelib/tape_util.h @@ -7,6 +7,6 @@ namespace tape { void sort(Tape &input, Tape &output); -} +} // namespace tape #endif diff --git a/src/filetape.cpp b/src/filetape.cpp index 8138e95..f50222d 100644 --- a/src/filetape.cpp +++ b/src/filetape.cpp @@ -1,12 +1,17 @@ #include "filetape.h" +#include #include using std::fstream; using tape::FileTape; -FileTape::FileTape(const std::string &file_name, FileTapeSettings settings) - : settings(settings) { - this->file = fstream(file_name); +std::string generate_tmp_file_name() { + std::array tmp_file_name{}; + // tmpnam is unsafe, but its cross-platform + // TODO: make a safe, cross-platform tmp file handler + // (possibly utilizing std::filesystem::temp_directory_path()) + std::tmpnam(tmp_file_name.data()); + return tmp_file_name.data(); } void fill_file_with_empty_cells(size_t size, fstream &file) { @@ -18,23 +23,25 @@ void fill_file_with_empty_cells(size_t size, fstream &file) { file.seekg(0, fstream::beg); } -FileTape::FileTape(size_t size, const std::string &file_name, +FileTape::FileTape(std::string file_name, FileTapeSettings settings) + : settings(settings), file_name(file_name) { + this->file = fstream(file_name); +} + +FileTape::FileTape(size_t size, std::string file_name, FileTapeSettings settings) : FileTape(file_name, settings) { this->file = - fstream(file_name, fstream::in | fstream::out | fstream::trunc); - fill_file_with_empty_cells(size, file); + fstream(this->file_name, fstream::in | fstream::out | fstream::trunc); + fill_file_with_empty_cells(size, this->file); } FileTape::FileTape(size_t size, FileTapeSettings settings) - : settings(settings) { - std::array file_name{}; - // tmpnam is unsafe, but its cross-platform - // create a platform dependant tmp file handler later - std::tmpnam(file_name.data()); + : settings(settings), temp(true) { + this->file_name = generate_tmp_file_name(); this->file = - fstream(file_name.data(), fstream::in | fstream::out | fstream::trunc); - fill_file_with_empty_cells(size, file); + fstream(this->file_name, fstream::in | fstream::out | fstream::trunc); + fill_file_with_empty_cells(size, this->file); } bool FileTape::seek_backwards() { @@ -79,3 +86,10 @@ void FileTape::write(uint32_t data) { this->file.write(cell.str().data(), FT_CELL_SIZE); this->file.seekg(-FT_SEEK_OFFSET + 1, fstream::cur); } + +FileTape::~FileTape() { + this->file.close(); + if (this->is_temp()) { + std::filesystem::remove(this->file_name); + } +} diff --git a/tests/assets/filetape_test_data.txt b/tests/assets/filetape_test_data.txt deleted file mode 100644 index 176a180..0000000 --- a/tests/assets/filetape_test_data.txt +++ /dev/null @@ -1,5 +0,0 @@ -0000000001 -0000000002 -0000000003 -0000000004 -0000000005 diff --git a/tests/filetape_tests.cpp b/tests/filetape_tests.cpp index 3f7cf25..4c12ba2 100644 --- a/tests/filetape_tests.cpp +++ b/tests/filetape_tests.cpp @@ -1,6 +1,18 @@ #include "filetape.h" #include +const static std::string TEST_DATA = "0000000001\n" + "0000012345\n" + "0000000000\n" + "2222222222\n" + "4294967295"; + +tape::FileTape prepare_test_filetape() { + tape::FileTape tape(0); + + return tape; +} + // NOLINTBEGIN(readability-function-cognitive-complexity) TEST_CASE("Reading data from a FileTape", "[filetape]") { tape::FileTape tape(FILETAPE_TEST_FILE);