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.

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

NameManager & const-correctness #277

Merged
merged 16 commits into from
Dec 12, 2014
Merged

NameManager & const-correctness #277

merged 16 commits into from
Dec 12, 2014

Conversation

mxgrey
Copy link
Member

@mxgrey mxgrey commented Nov 25, 2014

This commit contains a couple somewhat unrelated changes:

A NameManager class was created which has the ability to organize names of objects in a list and to generate new unique names if the user attempts to add an object with a name that is already being used.

The NameManager is being utilized by the World class to organize its Skeletons, and it's being used by the Skeleton class to organize its BodyNodes, SoftBodyNodes, Joints, and Markers.

This commit also contains const-correctness changes where accessor functions have been overloaded to offer a const version and a non-const version.

One final change was to create some consistency with out-of-bounds accessing behavior. Specifically, if the user requests an object from an index which is out-of-bounds, they will get back a NULL (which should probably be changed to "nullptr" in C++11 compatible versions of DART). This behavior already existed for many accessor functions, like Skeleton::getBodyNode(size_t), but it is now the standard for all other accessors, for example BodyNode::getCollisionShape(int).

@@ -0,0 +1,227 @@
/*
* Copyright (c) 2011-2014, Georgia Tech Research Corporation
Copy link
Member

Choose a reason for hiding this comment

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

The copyright year should begin with the year of the file created. For this file, just 2014 would be good.

@mxgrey
Copy link
Member Author

mxgrey commented Nov 26, 2014

Ah woops, didn't realize the dates were per-file. Fixed.

mNameBeforeNumber(true),
mPrefix(""), mInfix("("), mAffix(")") {}

std::string mDefaultName;
Copy link
Member

Choose a reason for hiding this comment

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

Member variable should be a protected member with some comment.

Copy link
Member Author

Choose a reason for hiding this comment

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

I actually left mDefaultName public on purpose, because there shouldn't be any negative consequences with the user having direct access to it. Is it a policy that absolutely all data members need to be wrapped in mutator/accessor functions?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, it's our convention that all data members need to be encapsulated.

Copy link
Member Author

Choose a reason for hiding this comment

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

Understood; I'll be sure to enforce that in the future.

@@ -624,9 +649,20 @@ class Skeleton

/// List of body nodes in the skeleton.
std::vector<BodyNode*> mBodyNodes;
const std::string& addEntryToBodyNodeNameMgr(BodyNode* _newNode);
Copy link
Member

Choose a reason for hiding this comment

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

Group member variables and member functions separately

@mxgrey
Copy link
Member Author

mxgrey commented Dec 1, 2014

The requested changes have been made. I also altered the NameManager so that it exclusively manages pointers to objects.

@jslee02 jslee02 added this to the Release DART (14.12) milestone Dec 11, 2014
@jslee02
Copy link
Member

jslee02 commented Dec 11, 2014

+1

karenliu added a commit that referenced this pull request Dec 12, 2014
NameManager & const-correctness
@karenliu karenliu merged commit dce738a into master Dec 12, 2014
This was referenced Jan 20, 2015
@mxgrey mxgrey deleted the grey/name_manager branch January 27, 2015 22:11
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.

3 participants