Epetra vector access operator #16880
Conversation
| namespace internal | ||
| { | ||
| /** | ||
| * Declare type for container size. | ||
| */ | ||
| using size_type = dealii::types::global_dof_index; | ||
|
|
||
| /** | ||
| * Declare type for container value type. | ||
| */ | ||
| using value_type = double; |
There was a problem hiding this comment.
Could we choose a namespace that makes sure these typedefs are only used for vectors? Say EpetraWrappers::internal::Vector or so?
There was a problem hiding this comment.
Hi @bangerth, I'm not sure if I got your comment correctly.
I just followed the same pattern used in other similar classes (Tpetra Vector, TrilinosWrappers::MPI::Vector and so on).
What exactly do you mean by choosing a different namespace for these aliases?
There was a problem hiding this comment.
Do you just want to drop aliases from the internal namespace and leave them only in the main Vector class?
There was a problem hiding this comment.
What I meant is that having
namespace EpetraWrappers {
namespace internal {
using value_type = double;
leaves a generically names type value_type in a namespace that is not clearly associated with the Vector class. It is perhaps true that all of the EpetraWrappers classes use double as their value type, but the point of the value_type you declare here is to support one specific class, and that should be made clear by either choosing a name that makes that obvious, or put it into a namespace that is clearly associated with the Vector class.
The ideal namespace, of course, is the class itself. So if there was a way to redirect all of the places where you use the typdef to the Vector::value_type type, that would be ideal.
Same for the other typedef (size_type).
There was a problem hiding this comment.
Oh, I see your point. Would something like this work?
I'm noticing a similar pattern in other places (e.g. in trilinos_vector.h and trilinos_tpetra_vector.h). If my proposed solution is ok, I'll take care of propagating the modification there.
pcafrica
force-pushed
the
epetra_vector_access_operator
branch
2 times, most recently
from
98210db
to
c356f59
Compare
|
Would you mind fix-up'ing the indent commit into the commit it is supposed to fix? |
pcafrica
force-pushed
the
epetra_vector_access_operator
branch
from
6d139c1
to
de0596c
Compare
Done! |
|
/rebuild |
|
Thank you -- good to go! |
This PR adresses - and hopefully solves - #16877.