dcmtk icon indicating copy to clipboard operation
dcmtk copied to clipboard

Provide a new API for DcmItem to delete a private attribute

Open malaterre opened this issue 4 years ago • 3 comments

Extend DcmItem with findAndDeletePrivateElement to allow user to delete a private tag directly (reservation number is computed internally).

malaterre avatar Aug 24 '21 11:08 malaterre

@eichelberg / @jriesmeier could you comment on this ? Bad code ? Should refactor existing findAndDeleteElement to take a DcmTag ? other ?

thx

malaterre avatar Jun 30 '25 10:06 malaterre

Thank you for your proposal. However in my opinion, a findAndDeletePrivateElement() helper function should not only support the "dynamic use" of private element numbers, but also something like (0011,1010,"MY PRIVATE CREATOR"), i.e. fully specified element numbers. A flag switching between both modes would also be nice.

Also, the call of "privTag.getElement() >= 0x1000" makes no sense if you want to check for attribute tags (gggg,00xx), where "xx" is between "00" and "FF".

Furthermore, I am not really convinced on how you've implemented the while loop, and that and how you call findAndDelete()...

jriesmeier avatar Jun 30 '25 16:06 jriesmeier

Hi Matthieu, since you explicitly asked for my feedback - I am also not happy with the PR as it is now:

  • The documentation talks about "deleting a private element", but apparently this is about passing the tag of a private creator element and then having the method delete that private creator and all associated private attributes. That is absolutely not clear from the API documentation.
  • The mapping from private creator element to related private elements is managed through getPrivateCreator(), which is not the right way to do things since the private creator strings are not guaranteed to be unique and since the relationship is actually defined through the element tag, so the code does the wrong thing in a rather inefficient way :-)
  • The searchIntoSub parameters makes no sense. Private elements in items have their own private creator elements and are not linked to one in the main dataset or a higher level (containing) sequence item

In general, however, I think a method that enables a user to "get rid" of a group of private elements by passing the tag of the private creator, or possibly the group number and the private creator string, would be useful to have.

eichelberg avatar Jul 01 '25 07:07 eichelberg