Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

MeshLib add properties #542

Merged
merged 40 commits into from
Jan 7, 2015
Merged

MeshLib add properties #542

merged 40 commits into from
Jan 7, 2015

Conversation

TomFischer
Copy link
Member

This PR is based on the discussion in PR #531:

  • It adds an implemention to manage properties within the Mesh.
  • The properties are associated to specific mesh items like mesh nodes or mesh
    elements via an enum class (see enum class MeshItemType).
  • The properties are not restricted to a particular data type, you can use several types like double, vector or matrix
    types at run time together (see tests).
  • There is not a possibility to get the value data type.

norihiro-w
norihiro-w reviewed Dec 4, 2014
View reviewed changes
MeshLib/Mesh.h
#include <boost/any.hpp>
#include <boost/optional.hpp>

#include "logog/include/logog.hpp"
Copy link
Collaborator

Choose a reason for hiding this comment

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

is logog used somewhere in this header? ✅

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point. It is not any more necessary to include logog.hpp, as well as boost/any.hpp, boost/optional.hpp and map. I will remove them.

@TomFischer TomFischer closed this Dec 5, 2014
@TomFischer TomFischer reopened this Dec 9, 2014
bilke
bilke reviewed Dec 11, 2014
View reviewed changes
MeshLib/Properties.h

/// @brief Material property manager on mesh items.
/// Class Properties manages scalar, vector or matrix properties. For instance
/// in groundwater flow porosity is scalar property and permeabilty can be
Copy link
Member

Choose a reason for hiding this comment

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

Typo: is _a_ scalar ✅

@bilke
Copy link
Member

bilke commented Dec 11, 2014

Good documentation! 👍

@TomFischer
Copy link
Member Author

Reworked according remarks of @bilke and rebased.

rinkk
rinkk reviewed Dec 15, 2014
View reviewed changes
MeshLib/Properties.h
/// on the heap), see the parameter item2group_mapping, is required.
/// @tparam T type of the property value
/// @param name the name of the property
/// @param mesh_item_type for instance node or element assigned properties
Copy link
Member

Choose a reason for hiding this comment

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

param-documentation for n_prop_groups and item2group_mapping are missing ✅

@TomFischer
[ML] Class Properties: add default constructor.
ef3f069
@TomFischer
[ML] PropertyVector: rename for better readability.
6eb8091
@TomFischer
[ML] Overload for Properties::newProperty() to add group properties.
e6ff905
@TomFischer
[ML] Property::getProperty(): returns const& instead of pointer.
a31e5b9
@TomFischer
[ML] Removed unused method addProperty from Properties.
dbf2622
@TomFischer
[T] Fixed tests.
22d61f2
@TomFischer
[ML] Properties: add method hasProperty().
f346860
@TomFischer
[ML] Docu for newProperty() methods.
9d843a5
@TomFischer
[ML] Properties: Update/add of docu.
6c40636
@TomFischer
[ML] PropertyVector: add docu.
f33804e
@TomFischer
[T] Testing Properties::hasProperty().
6b83d3a
@TomFischer
[ML] Corrected warning message.
3dc5932
@TomFischer
[ML] Made documentation correct / more clear.
a17902a
@TomFischer
[ML & T] More readable method name.
57db066
@TomFischer
[ML] Improve constness of variables in class Properties.
19c53bb
@TomFischer
[ML] Improved docu and error messages within the sources.
093b83d
@TomFischer
[ML] Check the mapping for the PropertyVector.
9d7d1ad
@TomFischer
[ML] Added const qualifier to Properties::getProperty().
0f24d35
@TomFischer
[ML] Class Properties is a friend of PropertyVector.
043fb04 
This commit prepares the moving the visibility of the constructor to the protected space.
@TomFischer
[ML] PropertyVector: Constructors are protected - only friends can ca…
7f4b89a 
…ll them.
@TomFischer
[ML] Properties: reuse comparison result.
2fb2b51
@TomFischer
[ML] Properties: exchange the checks (more efficient in error case).
b58d5ef
@norihiro-w
Copy link
Collaborator

👍

@endJunction
Copy link
Member

Very nice!

Two suggestions for future:

  • use make_pair, instead of pair ctor, which requires the types: std::make_pair(a, b) vs. std::pair<TheAType, TheBType>(a, b).
  • Don't hessitate to make a type alias for often used structures, like for the std::map<PropertyKey, boost::any> and its const_iterator making code more compact (and abstract).

I like very much the std::default_delete() usage. No memory leaks and no compilation warnings.

endJunction added a commit that referenced this pull request Jan 7, 2015
@endJunction
Merge pull request #542 from TomFischer/MeshLibAddProperties
8a51809 
Add mesh properties.
@endJunction endJunction merged commit 8a51809 into ufz:master Jan 7, 2015
@TomFischer TomFischer deleted the MeshLibAddProperties branch January 7, 2015 07:06
@ogsbot
Copy link
Member

ogsbot commented Jun 19, 2020

OpenGeoSys development has been moved to GitLab.

See this pull request on GitLab.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@TomFischer @bilke @rinkk @norihiro-w @endJunction @ogsbot