Skip to content

Remove shared pointer behaviour - #8

Open
OliverWoolland wants to merge 3 commits into
mainfrom
removed-shared-pointer
Open

Remove shared pointer behaviour#8
OliverWoolland wants to merge 3 commits into
mainfrom
removed-shared-pointer

Conversation

@OliverWoolland

Copy link
Copy Markdown
Contributor

No description provided.

Comment thread include/ro-crate.hpp
Comment on lines 231 to 247
inline void ROCrate::addEntity(const std::string& id, Entity& entity) {
// Validate the id (reject empty)
if (id.empty()) {
throw std::invalid_argument("Entity ID cannot be empty.");
}

// Check if the entity with the given id already exists in the register
if (entities_.find(id) != entities_.end()) {
throw std::runtime_error("Entity with id '" + id + "' already exists in the RO-Crate.");
}

// Add the assigned ID to the Entity itself
entity.assignId(id);

// Add an entity to the RO-Crate's entity register
entities_.emplace(id, entity);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I think I would do

inline void ROCrate::addEntity(const std::string& id, Entity entity) {

and

// Add an entity to the RO-Crate's entity register
entities_.emplace(id, std::move(entity));

It would end up with the same behaviour. Currently Entity is passed by reference and then copied into entities_. Passing by value makes it clear that the Entity stored in the crate is a separate object.

@jfecroftRSE jfecroftRSE left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Looks good. The example on the front page now also needs updating. Currently it makes copies of the root and rootData entities, then updates them which doesn't affect the ones in the crate

// Get the root metadata entity from the crate and set the description
//Entity root = crate.getEntity("ro-crate-metadata.json");
Entity& root = crate.getEntity("ro-crate-metadata.json");
root.set("description", "RO-Crate Metadata File Descriptor (this file)");

// Add name, description to the root data entity (./)
//Entity rootData = crate.getEntity("./");
Entity& rootData = crate.getEntity("./");
rootData.set("name", "Example RO-Crate");
rootData.set("description", "The RO-Crate Root Data Entity");

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants