Replace Lists with Vectors #970

IVlaD17 · 2022-02-10T13:14:53Z

Closes #619

Pretty much what's being said in the title. Replacing std::lists with std::vectors where possible.

Changed the following areas:

data1dstore.h
data2dstore.h
data3dstore.h
combopopulator.h
tablewidgetupdater.h
treewidgetupdater.h

… of pairs.

trisyoungs

Looks good. Some comments over a few of the loops which should be investigated.

trisyoungs · 2022-03-08T09:01:21Z

src/classes/data1dstore.cpp

+    return std::find_if(data_.begin()->get(), data_.end()->get(), [&name](auto &data) { return data.first.tag() == name; }) !=
+           data_.end()->get();


This doesn't look right and wouldn't traverse the vector:

Suggested change

return std::find_if(data_.begin()->get(), data_.end()->get(), [&name](auto &data) { return data.first.tag() == name; }) !=

data_.end()->get();

return std::find_if(data_.begin(), data_.end(), [&name](auto &data) { return data->first.tag() == name; }) !=

data_.end();

trisyoungs · 2022-03-08T09:02:48Z

src/classes/data1dstore.cpp

+    auto it = std::find_if(data_.begin()->get(), data_.end()->get(), [&name](auto &data) { return data.first.tag() == name; });
+    if (it == data_.end()->get())


Same here:

Suggested change

auto it = std::find_if(data_.begin()->get(), data_.end()->get(), [&name](auto &data) { return data.first.tag() == name; });

if (it == data_.end()->get())

auto it = std::find_if(data_.begin(), data_.end(), [&name](auto &data) { return data->first.tag() == name; });

if (it == data_.end())

trisyoungs · 2022-03-08T09:04:12Z

src/classes/data2dstore.cpp

+    return std::find_if(data_.begin()->get(), data_.end()->get(), [&name](auto &data) { return data.first.tag() == name; }) !=
+           data_.end()->get();


Suggested change

return std::find_if(data_.begin()->get(), data_.end()->get(), [&name](auto &data) { return data.first.tag() == name; }) !=

data_.end()->get();

auto it = std::find_if(data_.begin(), data_.end(), [&name](auto &data) { return data->first.tag() == name; });

if (it == data_.end())

trisyoungs · 2022-03-08T09:04:26Z

src/classes/data2dstore.cpp

+    auto it = std::find_if(data_.begin()->get(), data_.end()->get(), [&name](auto &data) { return data.first.tag() == name; });
+    if (it == data_.end()->get())


Suggested change

auto it = std::find_if(data_.begin()->get(), data_.end()->get(), [&name](auto &data) { return data.first.tag() == name; });

if (it == data_.end()->get())

auto it = std::find_if(data_.begin(), data_.end(), [&name](auto &data) { return data->first.tag() == name; });

if (it == data_.end())

trisyoungs · 2022-03-08T09:06:11Z

src/classes/data3dstore.cpp

+    return std::find_if(data_.begin()->get(), data_.end()->get(), [&name](auto &data) { return data.first.tag() == name; }) !=
+           data_.end()->get();


Suggested change

return std::find_if(data_.begin()->get(), data_.end()->get(), [&name](auto &data) { return data.first.tag() == name; }) !=

data_.end()->get();

auto it = std::find_if(data_.begin(), data_.end(), [&name](auto &data) { return data->first.tag() == name; });

if (it == data_.end())

trisyoungs · 2022-03-08T09:14:33Z

src/keywords/data3dstore.cpp

@@ -40,8 +40,9 @@ bool Data3DStoreKeyword::deserialise(LineParser &parser, int startArg, const Cor
 // Serialise data to specified LineParser
 bool Data3DStoreKeyword::serialise(LineParser &parser, std::string_view keywordName, std::string_view prefix) const
 {
-    for (const auto &[data, format] : data_.data())
+    for (const auto sharedDataPointer : data_.data())


Suggested change

for (const auto sharedDataPointer : data_.data())

for (const auto &sharedDataPointer : data_.data())

trisyoungs · 2022-03-08T09:14:46Z

src/modules/datatest/process.cpp

@@ -22,8 +22,9 @@ bool DataTestModule::process(Dissolve &dissolve, ProcessPool &procPool)
    Messenger::print("\n");

    // Loop over reference one-dimensional data supplied
-    for (auto &[referenceData, format] : test1DData_.data())
+    for (auto sharedDataPointer : test1DData_.data())


Suggested change

for (auto sharedDataPointer : test1DData_.data())

for (auto &sharedDataPointer : test1DData_.data())

trisyoungs · 2022-03-08T09:14:53Z

src/modules/datatest/process.cpp

@@ -63,8 +64,9 @@ bool DataTestModule::process(Dissolve &dissolve, ProcessPool &procPool)
    }

    // Loop over reference two-dimensional data supplied
-    for (auto &[referenceData, format] : test2DData_.data())
+    for (auto sharedDataPointer : test2DData_.data())


Suggested change

for (auto sharedDataPointer : test2DData_.data())

for (auto &sharedDataPointer : test2DData_.data())

trisyoungs · 2022-03-08T09:14:59Z

src/modules/rdf/functions.cpp

@@ -645,8 +645,9 @@ bool RDFModule::testReferencePartials(const Data1DStore &testData, double testTh
    LineParser parser;

    // Loop over supplied test data and see if we can locate it amongst our PartialSets
-    for (auto &[data, format] : testData.data())
+    for (auto sharedDataPointer : testData.data())


Suggested change

for (auto sharedDataPointer : testData.data())

for (auto &sharedDataPointer : testData.data())

trisyoungs · 2022-03-08T09:15:06Z

src/modules/rdf/functions.cpp

@@ -675,8 +676,9 @@ bool RDFModule::testReferencePartials(const Data1DStore &testData, double testTh
    LineParser parser;

    // Loop over supplied test data and see if we can locate it amongst our PartialSets
-    for (auto &[data, format] : testData.data())
+    for (auto sharedDataPointer : testData.data())


Suggested change

for (auto sharedDataPointer : testData.data())

for (auto &sharedDataPointer : testData.data())

rprospero · 2022-03-08T10:15:47Z

I agree with Tristan's comments, but otherwise approve.

IVlaD17 added 2 commits February 10, 2022 15:45

#619:Replaced std::lists with std::vectors in the DataStore classes.

2c96957

#619: Replaced std::list with std::vector in the gui helper methods.

95abce8

IVlaD17 force-pushed the 619-replace-lists-with-vectors branch from 12b6612 to 95abce8 Compare February 10, 2022 15:49

IVlaD17 added 7 commits February 15, 2022 15:46

#619: Attempt to fix compilation issues shown on the CI/CD

e1a5e23

#619: Updated the vectors to hold reference wrappers to pairs instead…

4f1c8be

… of pairs.

#619: Replaced the reference_wrappers with shared_ptrs.

7022c9f

#619: Fixed formatting issues.

41ed76a

#619: Fixed formatting issues again.

b53b20f

Merge branch 'develop' into 619-replace-lists-with-vectors

1036990

#619: Fixed the segfaults (hopefully)

3b82a16

IVlaD17 requested review from rprospero and trisyoungs March 7, 2022 15:44

trisyoungs requested changes Mar 8, 2022

View reviewed changes

IVlaD17 added 2 commits March 8, 2022 11:32

#619: Code tidy-ups according to Tristan's comments on the PR

ab64e66

#619: Fixed formatting problems

34ed26a

trisyoungs approved these changes Mar 10, 2022

View reviewed changes

IVlaD17 merged commit e0de92d into develop Mar 10, 2022

IVlaD17 deleted the 619-replace-lists-with-vectors branch March 10, 2022 09:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Replace Lists with Vectors #970

Replace Lists with Vectors #970

IVlaD17 commented Feb 10, 2022

trisyoungs left a comment

trisyoungs Mar 8, 2022

trisyoungs Mar 8, 2022

trisyoungs Mar 8, 2022

trisyoungs Mar 8, 2022

trisyoungs Mar 8, 2022

trisyoungs Mar 8, 2022

trisyoungs Mar 8, 2022

trisyoungs Mar 8, 2022

trisyoungs Mar 8, 2022

trisyoungs Mar 8, 2022

rprospero commented Mar 8, 2022

		return std::find_if(data_.begin()->get(), data_.end()->get(), [&name](auto &data) { return data.first.tag() == name; }) !=
		data_.end()->get();

		auto it = std::find_if(data_.begin()->get(), data_.end()->get(), [&name](auto &data) { return data.first.tag() == name; });
		if (it == data_.end()->get())

	for (const auto sharedDataPointer : data_.data())
	for (const auto &sharedDataPointer : data_.data())

	for (auto sharedDataPointer : test1DData_.data())
	for (auto &sharedDataPointer : test1DData_.data())

	for (auto sharedDataPointer : test2DData_.data())
	for (auto &sharedDataPointer : test2DData_.data())

	for (auto sharedDataPointer : testData.data())
	for (auto &sharedDataPointer : testData.data())

Replace Lists with Vectors #970

Replace Lists with Vectors #970

Conversation

IVlaD17 commented Feb 10, 2022

trisyoungs left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rprospero commented Mar 8, 2022