You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
The set_data method is implemented for the state_representation::State with three different argument types (vector, matrix, std::vector). To me it's somehow inconsistent to define the set_data in the base class but not the data method. I understand why it's not possible to do that - because you cant have different return types for the same function name.
I just feel it's weird to define it in a class where we said, that doesn't have any data field. If it would be a virtual method where we can at least reuse some of the code that's implemented in the base class, that would be okay. But in this case, all those methods just throw a NotImplemented exception so I'm not really sure what the benefit of that is.
I don't have strong feelings about this but maybe you feel a bit like me or you can give me a few reasons why it's good to have it in the base class? @eeberhard@buschbapti
The text was updated successfully, but these errors were encountered:
The
set_data
method is implemented for thestate_representation::State
with three different argument types (vector, matrix, std::vector). To me it's somehow inconsistent to define theset_data
in the base class but not thedata
method. I understand why it's not possible to do that - because you cant have different return types for the same function name.I just feel it's weird to define it in a class where we said, that doesn't have any data field. If it would be a virtual method where we can at least reuse some of the code that's implemented in the base class, that would be okay. But in this case, all those methods just throw a
NotImplemented
exception so I'm not really sure what the benefit of that is.I don't have strong feelings about this but maybe you feel a bit like me or you can give me a few reasons why it's good to have it in the base class? @eeberhard @buschbapti
The text was updated successfully, but these errors were encountered: