C++ - Object Oriented File System Design using Composite Design Pattern

Question

I am trying to come up with a good File System Design using C++ / OOP. This is being done to gain an understanding of System Design and Design Patterns.

I have some basic functionality, like reading and writing files, adding and deleting files and folders, calculating the size of files and folders. The design pattern I am trying to use is Composite Design pattern.

My design uses a base class called BaseComponent which contains facilities common to both files and folders. The class Folder deals with adding and deleting of files and folders while class File deals with reading and writing of files. Both File and Folder are derived from BaseComponent.

Here is the code:

#include <iostream> #include <string> #include <set> class BaseComponent { std::string name_v; BaseComponent* parent_v; bool is_folder; public: BaseComponent(std::string name, BaseComponent* parent = nullptr, bool f = true) : name_v{name}, parent_v{parent}, is_folder{f} { } virtual ~BaseComponent() { } void change_name(std::string& name) { name_v = name; } std::string name() const { return name_v; } BaseComponent* parent() const { return parent_v; } void del() { if(parent_v == nullptr) { return; } parent_v -> delete_component(this); } virtual int size() const { return 0; } virtual void add_component(BaseComponent*) { } virtual void delete_component(BaseComponent*) { } }; class Folder : public BaseComponent { std::set<BaseComponent*> children; public: Folder(std::string name, Folder* parent = nullptr, bool is_folder = true) : BaseComponent{name, parent, is_folder} { } ~Folder() { } int size() const { int size_v = 0; for(auto child : children) { size_v += child -> size(); } return size_v; } int num_ff() { return children.size(); } void add_component(BaseComponent* b) { children.insert(b); } void delete_component(BaseComponent* b) { children.erase(b); } }; class File : public BaseComponent { std::string contents_v; public: File(std::string contents, std::string name, Folder* parent, bool is_folder = false) : BaseComponent{name, parent, is_folder}, contents_v{contents} { } ~File() { } int size() const { return sizeof(this); } void write(std::string content) { contents_v = content; } std::string read() const { return contents_v; } }; /*class FileSystem { };*/ int main() { Folder root{"/"}; Folder home{"home", &root}; root.add_component(&home); Folder lib{"lib", &root}; root.add_component(&lib); Folder dev{"dev", &root}; root.add_component(&dev); std::cout << "No. of components in root: " << root.num_ff() << "\n"; std::cout << "No. of components in home: " << home.num_ff() << "\n"; File test{"x = 0, y = 0", "Configuration.txt", &root}; root.add_component(&test); Folder ws{"ws", &home}; home.add_component(&ws); File fs{"fs file", "fs.cpp", &ws}; ws.add_component(&fs); File git{"user: dummy", "git_config", &home}; home.add_component(&git); std::cout << "Contents of git: " << git.read() << "\n"; git.write("user: new\n email:[email protected]"); std::cout << "Contents of git: " << git.read() << "\n"; std::cout << "No. of components in root: " << root.num_ff() << "\n"; std::cout << "No. of components in home: " << home.num_ff() << "\n"; std::cout << "No. of components in ws: " << ws.num_ff() << "\n"; dev.del(); std::cout << "No. of components in root: " << root.num_ff() << "\n"; std::cout << "Size of git: " << git.size() << "\n"; std::cout << "Size of root: " << root.size() << "\n"; return 0; } 

1. The add_component is being called from main. How can I change that? I don't mind calling it inside a new class called FileSystem by creating functions such as mkdir or mkfile to handle folder and file creation. But I am not able to come up with a good design.

2. If FileSystem is being implemented, I would like to avoid having to take the pointer to parent inside the main, while creating files and folders. This is again something that is stopping me from coming up with a better design.

3. How to improve the functionalities implemented? Are there any significant issues in this part?

4. Is a better design possible without changing classes and their interface?

5. Kindly provide an overall review.

Answer 1

Something that jumps out immediately is the is_folder member. It's a bad sign if a base class needs to know what subclass it is. Even more concerning, the subclass constructors allow one to create a File with is_folder set, or a Folder with is_folder reset. Thankfully, this member seems never to be used, so it can be omitted altogether.

---

I think that BaseComponent ought to be an abstract class. A simple way to do so is to make size() a pure virtual method:

virtual std::size_t size() const = 0; 

(I've used a more appropriate return type, too - though some platforms support files larger than addressable memory, so be careful).

---

Similarly, the destructor that's declared just to make it virtual (well done for remembering) can be defined =default:

virtual ~BaseComponent() = default; 

The subclass destructors can be omitted, as the compiler-generated ones will be exactly equivalent.

---

We can save the two-step construct-then-add by having the constructor also add to the parent entry:

BaseComponent::BaseComponent(std::string name, Folder* parent) : name_v{std::move(name)}, parent_v{parent} { if (parent) { parent->add_component(this); } } 

It also makes sense for a destructor to remove the object from its parent (otherwise we could end up with dangling references), and for a deleted Folder to null out its children's parent pointers.

---

We can help avoid mistakes by using the override keyword when we reimplement size() in the subclasses:

std::size_t size() const override { return sizeof this; } 

BTW, I hope that sizeof this is just a placeholder for something more useful later (all pointers to File objects will have the same size on any given system). You probably meant return contents_v.size(); there.

---

Minor points:

• num_ff() should be const
• Constructor arguments passed by value should be std::move()d to their new homes.
• BaseComponent constructor and destructor can be protected. Consider suppressing copy construction and copy assignment.
• BaseComponent isn't a very informative name.

---

Modified code

#include <numeric> #include <string> #include <set> class Folder; class File; class FileSystemObject { std::string name_v; Folder* parent_v; protected: FileSystemObject(std::string name, Folder* parent); FileSystemObject(const FileSystemObject&) = delete; FileSystemObject& operator=(const FileSystemObject&) = delete; virtual ~FileSystemObject(); public: void change_name(std::string& name) { name_v = name; } std::string name() const { return name_v; } Folder* parent() const { return parent_v; } virtual std::size_t size() const = 0; void set_parent(Folder* parent); }; class Folder : public FileSystemObject { std::set<FileSystemObject*> children = {}; public: Folder(std::string name, Folder* parent = nullptr) : FileSystemObject{std::move(name), parent} { } ~Folder() { for (auto c: children) { c->set_parent(nullptr); } } std::size_t size() const override { return std::accumulate(children.begin(), children.end(), 0, [](auto sum, auto child){ return sum + child->size(); }); } int num_ff() const { return children.size(); } void add_component(FileSystemObject* b) { children.insert(b); } void delete_component(FileSystemObject* b) { children.erase(b); } }; class File : public FileSystemObject { std::string contents_v; public: File(std::string contents, std::string name, Folder* parent) : FileSystemObject{name, parent}, contents_v{contents} { } std::size_t size() const override { return contents_v.size(); } void write(std::string content) { contents_v = std::move(content); } std::string read() const { return contents_v; } }; FileSystemObject::FileSystemObject(std::string name, Folder* parent) : name_v{std::move(name)}, parent_v{parent} { if (parent) { parent->add_component(this); } } FileSystemObject::~FileSystemObject() { if (parent_v) { parent_v->delete_component(this); } } void FileSystemObject::set_parent(Folder* parent) { if (parent_v) { parent_v->delete_component(this); } if ((parent_v = parent)) { // assignment! parent_v->add_component(this); } } #include <iostream> int main() { Folder root{"/"}; Folder home{"home", &root}; Folder lib{"lib", &root}; Folder dev{"dev", &root}; std::cout << "No. of components in root: " << root.num_ff() << '\n'; std::cout << "No. of components in home: " << home.num_ff() << '\n'; File test{"x = 0, y = 0", "Configuration.txt", &root}; Folder ws{"ws", &home}; File fs{"fs file", "fs.cpp", &ws}; File git{"user: dummy", "git_config", &home}; std::cout << "Contents of git: " << git.read() << '\n'; git.write("user: new\n email:[email protected]"); std::cout << "Contents of git: " << git.read() << '\n'; std::cout << "No. of components in root: " << root.num_ff() << '\n'; std::cout << "No. of components in home: " << home.num_ff() << '\n'; std::cout << "No. of components in ws: " << ws.num_ff() << '\n'; dev.set_parent(nullptr); std::cout << "No. of components in root: " << root.num_ff() << '\n'; std::cout << "Size of git: " << git.size() << '\n'; std::cout << "Size of root: " << root.size() << '\n'; }