feat: add toBuilder() methods
#4796
Conversation
paullatzelsperger
force-pushed
the
feat/add_toBuilder_methods
branch
from
14a91c5 to
4a1f820
Compare
paullatzelsperger
force-pushed
the
feat/add_toBuilder_methods
branch
from
4a1f820 to
6d19042
Compare
paullatzelsperger
force-pushed
the
feat/add_toBuilder_methods
branch
from
6d19042 to
19b158d
Compare
| @@ -119,6 +119,10 @@ public DataModelVersion getDataModelVersion() { | |||
| return dataModelVersion; | |||
| } | |||
|
|
|||
| public <T extends VerifiableCredential, B extends Builder<T, B>> Builder<T, B> toBuilder() { | |||
| return new Builder(this); | |||
There was a problem hiding this comment.
we must be aware with this approach (that differs from the current toBuilder implementations we have e.g. in Policy or Asset instances), that the changes applied on the builder will affect the original object instance.
Maybe we should decide on a standard approach about builders, because the different approaches used currently could cause confusion.
There was a problem hiding this comment.
excellent point.
as far as i can see, both Asset and Policy affect the original instance (i.e. modify some related objects) but don't directly operate on it, whereas the toBuilders implemented here directly modify the original instance. subtle but an important difference.
to me, the implementation here is more correct, because it directly operates on the original instance.
as an addition, we could introduce a copy() method that creates a carbon-copy. WDYT?
[edit]: if you agree, i could adapt Policy#toBuilder and Asset#toBuilder accordingly
There was a problem hiding this comment.
maybe we could state a clear decision and change all the builders appropriately and to document it, I'm not sure if there are other builders that behave as Asset and Policy, but I guess so
There was a problem hiding this comment.
i'll open an issue about this: #4797
What this PR changes/adds
adds
toBuilder()methods to several VC model classesWhy it does that
to enable easy copying, and setting values. this is needed when updating credentials, e.g. status list credentials in IH
Further notes
List other areas of code that have changed but are not necessarily linked to the main feature. This could be method
signature changes, package declarations, bugs that were encountered and were fixed inline, etc.
Who will sponsor this feature?
Please @-mention the committer that will sponsor your feature.
Linked Issue(s)
Closes # <-- insert Issue number if one exists
Please be sure to take a look at the contributing guidelines and our etiquette for pull requests.