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

Replace std::string with string #1301

Merged
merged 2 commits into from
Jan 30, 2021
Merged

Replace std::string with string #1301

merged 2 commits into from
Jan 30, 2021

Conversation

rrsettgast
Copy link
Member

This PR replaces instances of std::string with the string alias inside of GEOSX.

@TotoGaz
Copy link
Contributor

TotoGaz commented Jan 28, 2021

You may be willing to remove the #include <string> too? There are still a bunch of them.

@rrsettgast
Copy link
Member Author

You may be willing to remove the #include <string> too? There are still a bunch of them.

Oh. Yes.

@TotoGaz
Copy link
Contributor

TotoGaz commented Jan 28, 2021

There are a couple of occurrences of std::to_string; don't know what to do with it though.

@@ -29,7 +29,7 @@ namespace stringutilities
/**
* String tokenizing function
**/
string_array Tokenize( const std::string & str, const std::string & delimiters )
string_array Tokenize( const string & str, const string & delimiters )
Copy link
Contributor

@sytuannguyen sytuannguyen Jan 29, 2021

Choose a reason for hiding this comment

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

You may want to change also "const string" to "string const" as the rule said:

image

Copy link
Contributor

@TotoGaz TotoGaz Jan 29, 2021

Choose a reason for hiding this comment

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

Position of const is not restrained to string. It will probably be more efficient to deal with const position all at once, for all the argument types, in another PR, when we find/build an automated way to do this.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes....we should do the const enforcement in a stand-alone PR.

@sytuannguyen
Copy link
Contributor

sytuannguyen commented Jan 29, 2021

Do you want to take also a look at the types: int and integer? Are they both related to the aliases localIndex/globalIndex?

@rrsettgast
Copy link
Member Author

rrsettgast commented Jan 29, 2021

Do you want to take also a look at the types: int and integer? Are they both related to the aliases localIndex/globalIndex?

If they are actually a local or global index, then that they should be localIndex or globalIndex. Otherwise if they are an integer data type in the repository, they should be integer. If they are in a kernel, they should be int unless it is an index into an ArrayView.

Whatever the case...this PR is to fix string and only to fix string

@rrsettgast
Copy link
Member Author

There are a couple of occurrences of std::to_string; don't know what to do with it though.

I think we keep them. If we ever change the string alias to something other than std::string we will have to make sure there is a UDC to std::string.

@rrsettgast rrsettgast merged commit 5c37f01 into develop Jan 30, 2021
@rrsettgast rrsettgast deleted the cleanup/constAndString branch January 30, 2021 06:51
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