Skip to content

Commit

Permalink
Rename JFactoryT::GetAndCreate() to CreateAndGetData
Browse files Browse the repository at this point in the history 
The new name more accurately reflects the relationship between Get() and Create(). The caching logic always lives inside Create(), so that untyped retrievals via podio::Frame or JFactoryT::GetAs behave consistently.
  • Loading branch information
@nathanwbrei
nathanwbrei committed Apr 15, 2024
1 parent 22b57be commit ff61223
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 26 deletions.
18 changes: 9 additions & 9 deletions src/libraries/JANA/JEvent.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -295,7 +295,7 @@ inline JMetadata<T> JEvent::GetMetadata(const std::string& tag) const {

auto factory = GetFactory<T>(tag, true);
// Make sure that JFactoryT::Process has already been called before returning the metadata
factory->GetOrCreate(this->shared_from_this());
factory->CreateAndGetData(this->shared_from_this());
return factory->GetMetadata();
}

Expand All @@ -314,7 +314,7 @@ JFactoryT<T>* JEvent::Get(const T** destination, const std::string& tag) const
{
auto factory = GetFactory<T>(tag, true);
JCallGraphEntryMaker cg_entry(mCallGraph, factory); // times execution until this goes out of scope
auto iterators = factory->GetOrCreate(this->shared_from_this());
auto iterators = factory->CreateAndGetData(this->shared_from_this());
if (std::distance(iterators.first, iterators.second) == 0) {
*destination = nullptr;
}
Expand All @@ -330,7 +330,7 @@ JFactoryT<T>* JEvent::Get(std::vector<const T*>& destination, const std::string&
{
auto factory = GetFactory<T>(tag, true);
JCallGraphEntryMaker cg_entry(mCallGraph, factory); // times execution until this goes out of scope
auto iterators = factory->GetOrCreate(this->shared_from_this());
auto iterators = factory->CreateAndGetData(this->shared_from_this());
for (auto it=iterators.first; it!=iterators.second; it++) {
destination.push_back(*it);
}
Expand All @@ -350,7 +350,7 @@ JFactoryT<T>* JEvent::Get(std::vector<const T*>& destination, const std::string&
template<class T> const T* JEvent::GetSingle(const std::string& tag) const {
auto factory = GetFactory<T>(tag, true);
JCallGraphEntryMaker cg_entry(mCallGraph, factory); // times execution until this goes out of scope
auto iterators = factory->GetOrCreate(this->shared_from_this());
auto iterators = factory->CreateAndGetData(this->shared_from_this());
if (std::distance(iterators.first, iterators.second) == 0) {
return nullptr;
}
Expand All @@ -366,7 +366,7 @@ template<class T> const T* JEvent::GetSingle(const std::string& tag) const {
template<class T> const T* JEvent::GetSingleStrict(const std::string& tag) const {
auto factory = GetFactory<T>(tag, true);
JCallGraphEntryMaker cg_entry(mCallGraph, factory); // times execution until this goes out of scope
auto iterators = factory->GetOrCreate(this->shared_from_this());
auto iterators = factory->CreateAndGetData(this->shared_from_this());
if (std::distance(iterators.first, iterators.second) == 0) {
JException ex("GetSingle failed due to missing %d", NAME_OF(T));
ex.show_stacktrace = false;
Expand All @@ -386,7 +386,7 @@ std::vector<const T*> JEvent::Get(const std::string& tag) const {

auto factory = GetFactory<T>(tag, true);
JCallGraphEntryMaker cg_entry(mCallGraph, factory); // times execution until this goes out of scope
auto iters = factory->GetOrCreate(this->shared_from_this());
auto iters = factory->CreateAndGetData(this->shared_from_this());
std::vector<const T*> vec;
for (auto it=iters.first; it!=iters.second; ++it) {
vec.push_back(*it);
Expand Down Expand Up @@ -415,7 +415,7 @@ template<class T>
void JEvent::GetAll(std::vector<const T*>& destination) const {
auto factories = GetFactoryAll<T>(true);
for (auto factory : factories) {
auto iterators = factory->GetOrCreate(this->shared_from_this());
auto iterators = factory->CreateAndGetData(this->shared_from_this());
for (auto it = iterators.first; it != iterators.second; it++) {
destination.push_back(*it);
}
Expand All @@ -429,7 +429,7 @@ std::vector<const T*> JEvent::GetAll() const {
auto factories = GetFactoryAll<T>(true);

for (auto factory : factories) {
auto iters = factory->GetOrCreate(this->shared_from_this());
auto iters = factory->CreateAndGetData(this->shared_from_this());
std::vector<const T*> vec;
for (auto it = iters.first; it != iters.second; ++it) {
vec.push_back(*it);
Expand Down Expand Up @@ -462,7 +462,7 @@ template<class T>
typename JFactoryT<T>::PairType JEvent::GetIterators(const std::string& tag) const {
auto factory = GetFactory<T>(tag, true);
JCallGraphEntryMaker cg_entry(mCallGraph, factory); // times execution until this goes out of scope
auto iters = factory->GetOrCreate(this->shared_from_this());
auto iters = factory->CreateAndGetData(this->shared_from_this());
return iters;
}

Expand Down
4 changes: 2 additions & 2 deletions src/libraries/JANA/JFactoryT.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,11 +74,11 @@ class JFactoryT : public JFactory {
return mData.size();
}

/// GetOrCreate handles all the preconditions and postconditions involved in calling the user-defined Open(),
/// CreateAndGetData handles all the preconditions and postconditions involved in calling the user-defined Open(),
/// ChangeRun(), and Process() methods. These include making sure the JFactory JApplication is set, Init() is called
/// exactly once, exceptions are tagged with the originating plugin and eventsource, ChangeRun() is
/// called if and only if the run number changes, etc.
PairType GetOrCreate(const std::shared_ptr<const JEvent>& event) {
PairType CreateAndGetData(const std::shared_ptr<const JEvent>& event) {
Create(event);
return std::make_pair(mData.cbegin(), mData.cend());
}
Expand Down
30 changes: 15 additions & 15 deletions src/programs/unit_tests/JFactoryTests.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,27 +12,27 @@
TEST_CASE("JFactoryTests") {


SECTION("GetOrCreate calls Init, ChangeRun, and Process as needed") {
SECTION("CreateAndGetData calls Init, ChangeRun, and Process as needed") {

JFactoryTestDummyFactory sut;
auto event = std::make_shared<JEvent>();

// The first time it is run, Init, ChangeRun, and Process each get run once
sut.GetOrCreate(event);
sut.CreateAndGetData(event);
REQUIRE(sut.init_call_count == 1);
REQUIRE(sut.change_run_call_count == 1);
REQUIRE(sut.process_call_count == 1);

// We can getOrCreate as many times as we want and nothing will happen
// until somebody clears the factory (assuming persistence flag is unset)
sut.GetOrCreate(event);
sut.CreateAndGetData(event);
REQUIRE(sut.init_call_count == 1);
REQUIRE(sut.change_run_call_count == 1);
REQUIRE(sut.process_call_count == 1);

// Once we clear the factory, Process gets called again but Init and ChangeRun do not.
sut.ClearData();
sut.GetOrCreate(event);
sut.CreateAndGetData(event);
REQUIRE(sut.init_call_count == 1);
REQUIRE(sut.change_run_call_count == 1);
REQUIRE(sut.process_call_count == 2);
Expand Down Expand Up @@ -62,31 +62,31 @@ TEST_CASE("JFactoryTests") {
// For the first event, ChangeRun() always gets called
event->SetEventNumber(1);
event->SetRunNumber(22);
sut.GetOrCreate(event);
sut.CreateAndGetData(event);
REQUIRE(sut.change_run_call_count == 1);

// Subsequent events with the same run number do not trigger ChangeRun()
event->SetEventNumber(2);
sut.ClearData();
sut.GetOrCreate(event);
sut.CreateAndGetData(event);

event->SetEventNumber(3);
sut.ClearData();
sut.GetOrCreate(event);
sut.CreateAndGetData(event);
REQUIRE(sut.change_run_call_count == 1);

// As soon as the run number changes, ChangeRun() gets called again
event->SetEventNumber(4);
event->SetRunNumber(49);
sut.ClearData();
sut.GetOrCreate(event);
sut.CreateAndGetData(event);
REQUIRE(sut.change_run_call_count == 2);

// This keeps happening
event->SetEventNumber(5);
event->SetRunNumber(6180);
sut.ClearData();
sut.GetOrCreate(event);
sut.CreateAndGetData(event);
REQUIRE(sut.change_run_call_count == 3);
}

Expand All @@ -98,7 +98,7 @@ TEST_CASE("JFactoryTests") {
sut.ClearFactoryFlag(JFactory::NOT_OBJECT_OWNER);
sut.Insert(new JFactoryTestDummyObject(42, &deleted_flag));
sut.ClearData();
auto results = sut.GetOrCreate(event);
auto results = sut.CreateAndGetData(event);
REQUIRE(std::distance(results.first, results.second) == 0);
REQUIRE(deleted_flag == true);
}
Expand All @@ -111,7 +111,7 @@ TEST_CASE("JFactoryTests") {
sut.SetFactoryFlag(JFactory::NOT_OBJECT_OWNER);
sut.Insert(new JFactoryTestDummyObject(42, &deleted_flag));
sut.ClearData();
auto results = sut.GetOrCreate(event);
auto results = sut.CreateAndGetData(event);
REQUIRE(std::distance(results.first, results.second) == 0);
REQUIRE(deleted_flag == false);
}
Expand All @@ -124,7 +124,7 @@ TEST_CASE("JFactoryTests") {
sut.ClearFactoryFlag(JFactory::NOT_OBJECT_OWNER);
sut.Insert(new JFactoryTestDummyObject(42, &deleted_flag));
sut.ClearData();
auto results = sut.GetOrCreate(event);
auto results = sut.CreateAndGetData(event);
REQUIRE(std::distance(results.first, results.second) == 1);
REQUIRE(deleted_flag == false);
}
Expand All @@ -137,7 +137,7 @@ TEST_CASE("JFactoryTests") {
sut.SetFactoryFlag(JFactory::NOT_OBJECT_OWNER);
sut.Insert(new JFactoryTestDummyObject(42, &deleted_flag));
sut.ClearData();
auto results = sut.GetOrCreate(event);
auto results = sut.CreateAndGetData(event);
REQUIRE(std::distance(results.first, results.second) == 1);
REQUIRE(deleted_flag == false);
}
Expand All @@ -160,7 +160,7 @@ TEST_CASE("JFactoryTests") {

Issue135Factory sut;
auto event = std::make_shared<JEvent>();
auto results = sut.GetOrCreate(event);
auto results = sut.CreateAndGetData(event);
REQUIRE(sut.GetNumObjects() == 3);
REQUIRE(std::distance(results.first, results.second) == 3);

Expand Down Expand Up @@ -192,7 +192,7 @@ TEST_CASE("JFactoryTests") {

REQUIRE(sut.GetStatus() == JFactory::Status::Inserted);

auto results = sut.GetOrCreate(event);
auto results = sut.CreateAndGetData(event);
auto it = results.first;
REQUIRE((*it)->data == 49);
REQUIRE(sut.GetNumObjects() == 1);
Expand Down

0 comments on commit ff61223

Please sign in to comment.