Open carpawell opened 2 years ago
var obj object.Object
obj.SetExpirationEpoch(123)
// is much better to me than
obj.SetAttribute(object.AttrExpirationEpoch, strconv.FormatUint(123, 10))
duplicated call may be resolved differently (panic or leave on the user conscience) and almost definitely unexpected
dedicated method also takes care about format and prevents calls like
obj.SetAttribute(object.AttrExpirationEpoch, "1.5")
My concerns are mostly around compatibility (there is old code doing SetAttribute(s)
) and API consistency (interaction between this and SetAttribute(s)
). If it can be explained in some meaningful way to the user. But this would also mean that a similar API is expected of other system attributes.
@smallhive?
I consider using separate methods for system/predefined attributes to be more good, than evil. It will help cover some situations, like obj.SetAttribute(object.AttrExpirationEpoch, "1.5")
, because we may define an expected type of value. Such moments are important for us rather than for the user. We want to have properly filled and named system attributes.
How many system attributes do we have? Ten, maybe up to twenty, if I remember correctly. Adding new methods will improve object API. Old code is going to work as is, all of them are still available to set via obj.SetAttribute
method.
According to the documentation - we should update SetAttribute
docs and attract attention to the fact this method is more about user-defined attributes. The method also allows putting system attributes as well, but the preferable way is using separate methods and we should leave the method list
Now we use attrs and constants in CLI and some other places. I think version-independent setter and getter could be helpful.
Think also could be applied to the version-depended repo.