Conversation
Useful for templates
Allows to write code for Field3D, that also works for Field2D
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
include/bout/field3d.hxx
Outdated
|
|
||
| std::weak_ptr<Options> getTracking() { return tracking; }; | ||
|
|
||
| bool allowCalcParallelSlices{true}; |
There was a problem hiding this comment.
warning: member variable 'allowCalcParallelSlices' has public visibility [cppcoreguidelines-non-private-member-variables-in-classes]
bool allowCalcParallelSlices{true};
^There was a problem hiding this comment.
Could we add a public getter (and setter) instead of making the member variable public?
ZedThree
left a comment
ZedThree
left a comment
There was a problem hiding this comment.
Nice, thanks @dschwoerer! I guess my main question is could/should the new methods actually be virtual? Then we could just have a default empty implementation on Field.
include/bout/field2d.hxx
Outdated
| void calcParallelSlices() const {} | ||
| void splitParallelSlices() const {} | ||
| void clearParallelSlices() const {} | ||
| int numberParallelSlices() const { return 0; } |
There was a problem hiding this comment.
Should these be virtual? Or do they only need to be dummy methods?
include/bout/field3d.hxx
Outdated
|
|
||
| std::weak_ptr<Options> getTracking() { return tracking; }; | ||
|
|
||
| bool allowCalcParallelSlices{true}; |
There was a problem hiding this comment.
Could we add a public getter (and setter) instead of making the member variable public?
| std::weak_ptr<Options> getTracking() { return tracking; }; | ||
|
|
||
| bool allowCalcParallelSlices() const { return _allowCalcParallelSlices; }; | ||
| void disallowCalcParallelSlices() { _allowCalcParallelSlices = false; }; |
There was a problem hiding this comment.
What do these do? It doesn't look like they're used anywhere.
Also they sound like they should set _allowCalcParallelSlices to true/false, but the first one is a getter and the second one a setter.
Maybe the first should be areCalcParallelSlicesAllowed()?
There was a problem hiding this comment.
What do these do? It doesn't look like they're used anywhere.
They slipped in from a different PR.
Also they sound like they should set
_allowCalcParallelSlicestotrue/false, but the first one is a getter and the second one a setter.Maybe the first should be
areCalcParallelSlicesAllowed()?
Will change it 👍
| /// Check if this field has yup and ydown fields | ||
| /// Return reference to yup field | ||
| Field3D& yup(std::vector<Field3D>::size_type index = 0) { | ||
| Field3D& yup(size_t index = 0) { |
There was a problem hiding this comment.
I would've thought we would also need these to be virtual to make things properly generic? Although I guess you're mostly interested in FieldMetric and compile-time polymorphism really, and not runtime polymorphism?
We could make these virtual by returning Field&, but we probably don't actually want that.
There was a problem hiding this comment.
Yes, that is the reason. We do not want to return Field, so there is no way to make this virtual.
Compile time polymorphism is right now sufficient, as long as 3D metrics are a compile time choice.
3 participants
Make it easier to write FCI compatible code.