#Modified code
Modified code
#Modified code
Modified code
Minor points:
Minor points
Minor points:
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 beconst
- Constructor arguments passed by value should be
std::move()
d to their new homes. BaseComponent
constructor and destructor can beprotected
. 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';
}