Skip to content

[WIP] 480 refactor vr#483

Open
luissantosHCIT wants to merge 22 commits into
dcmjs-org:masterfrom
luissantosHCIT:480_refactor_vr
Open

[WIP] 480 refactor vr#483
luissantosHCIT wants to merge 22 commits into
dcmjs-org:masterfrom
luissantosHCIT:480_refactor_vr

Conversation

@luissantosHCIT

Copy link
Copy Markdown

See #480.

This is my attempt at addressing issues I identified with ValueRepresentation as well as code cleanup.
I am also creating a test harness here to ensure key methods and ValueRepresentation types maintain the expected behavior pre and post refactor. Perhaps, those test will help others confidently refactor VR further in the future.

…ected behavior.

Signed-off-by: Luis M. Santos <lsantos@medicalmasses.com>
…to refactoring.

Signed-off-by: Luis M. Santos <lsantos@medicalmasses.com>
…itly accept the value parameter.

Signed-off-by: Luis M. Santos <lsantos@medicalmasses.com>
Signed-off-by: Luis M. Santos <lsantos@medicalmasses.com>
…multiplicity.

Signed-off-by: Luis M. Santos <lsantos@medicalmasses.com>
… with multiplicity.

Signed-off-by: Luis M. Santos <lsantos@medicalmasses.com>
@netlify

netlify Bot commented Mar 11, 2026

Copy link
Copy Markdown

Deploy Preview for dcmjs2 ready!

Name Link
🔨 Latest commit dbabdb5
🔍 Latest deploy log https://app.netlify.com/projects/dcmjs2/deploys/69b1e38b0bf2440007ea6cbe
😎 Deploy Preview https://deploy-preview-483--dcmjs2.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

…rpose of the current writeBytes method. This method should be renamed to reflect its validation role better.

Signed-off-by: Luis M. Santos <lsantos@medicalmasses.com>
…r and consistent the selection of the padding character at the base VR class level. It's all based on the VR type anyways.

Signed-off-by: Luis M. Santos <lsantos@medicalmasses.com>
…ic behind generating multiplicity write length and its validity/verification.

Signed-off-by: Luis M. Santos <lsantos@medicalmasses.com>
…se the VR's padded versions since they handle dropping off the padding byte from the returned output thus making reads consistent with the writes.

Signed-off-by: Luis M. Santos <lsantos@medicalmasses.com>
… the specialized VR's read/write methods and direct reads from the stream. The latter is present to check that the generated raw value is correct. The former is there to validate that f(x) => y && g(y) => x in terms of the read/write API.

Signed-off-by: Luis M. Santos <lsantos@medicalmasses.com>
@luissantosHCIT

Copy link
Copy Markdown
Author

As I update the unit test harness, I realized that to test the readEncodedString family of methods #455 will need to be merged in or the new tests will fail in the future due to the broken encoding/decoding before that PR. I do not directly invoke this method but I made a refactor to use the padded version in the ValueRepresentation class which handles the pad byte more gracefully.

…on prior change.

Signed-off-by: Luis M. Santos <lsantos@medicalmasses.com>
Signed-off-by: Luis M. Santos <lsantos@medicalmasses.com>
Signed-off-by: Luis M. Santos <lsantos@medicalmasses.com>
…new writeBytes.

Signed-off-by: Luis M. Santos <lsantos@medicalmasses.com>
… value is emitted.

Signed-off-by: Luis M. Santos <lsantos@medicalmasses.com>
…ling reset resets this value even if the stream contents and size remain. As a result, an error is thrown inappropriately.

Signed-off-by: Luis M. Santos <lsantos@medicalmasses.com>
…ssed down to writeBytes and properly pass validity for unbounded buffers.

Signed-off-by: Luis M. Santos <lsantos@medicalmasses.com>
…pes. Unfortunately, these types write directly via a writeBytes override and thus I make sure here that we use that method when testing binary buffers.

Signed-off-by: Luis M. Santos <lsantos@medicalmasses.com>
… size should be for a binary vr. The new way addresses an issue with the dicom dict instance ending up with the improper maxLength on construction in data-tests.

Signed-off-by: Luis M. Santos <lsantos@medicalmasses.com>
… the purposes of reading a file. This accommodates the way data-tests checks against raw values. I agree that if a user wants to force a raw read, they should.

Signed-off-by: Luis M. Santos <lsantos@medicalmasses.com>
… raw value when accounting for oddness and pad byte value.

Signed-off-by: Luis M. Santos <lsantos@medicalmasses.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant