Invalid item in loop over sequences

Questions regarding the DCMRT library, a DCMTK add-on that implements support for the various DICOM Radiation Therapy (RT) IODs

Moderator: Moderator Team

Post Reply
Message
Author
nbeck
Posts: 8
Joined: Fri, 2022-10-28, 12:56

Invalid item in loop over sequences

#1 Post by nbeck »

Hello!

i've stumbled across some unexpected error messages today, when performing loops over sequences of the following kind:

Code: Select all

	do
	{
		DRTROIContourSequence::Item& currentContour = rtROIContourSequence.getCurrentItem();
		if (currentContour.isValid())
		{
			// doSomething
		}
		else {
			std::cerr << "Error: Invalid item "<< std::endl;
		}
	} while (rtROIContourSequence.gotoNextItem().good());
The loop always yields one invalid item before exiting.

It appears that there is a problem in the gotoNextItem() method.

Code: Select all

OFCondition DRTROIContourSequence::gotoNextItem()
{
    OFCondition result = EC_IllegalCall;
    if (CurrentItem != SequenceOfItems.end())
    {
        ++CurrentItem;
        result = EC_Normal;
    }
    return result;
}
I believe, the iterator should be incremented before comparing against SequenceOfItems.end(). Any thoughts?

Note that I am using the standard library instead of the built-in types of DCMTK.

J. Riesmeier
DCMTK Developer
Posts: 2501
Joined: Tue, 2011-05-03, 14:38
Location: Oldenburg, Germany
Contact:

Re: Invalid item in loop over sequences

#2 Post by J. Riesmeier »

Thank you for the report. We have to check this and will get back to you then.

J. Riesmeier
DCMTK Developer
Posts: 2501
Joined: Tue, 2011-05-03, 14:38
Location: Oldenburg, Germany
Contact:

Re: Invalid item in loop over sequences

#3 Post by J. Riesmeier »

Could you please check whether the following solves the problem?

Code: Select all

OFCondition DRTROIContourSequence::gotoNextItem()
{
    OFCondition result = EC_IllegalCall;
    if (++CurrentItem != SequenceOfItems.end())
    {
        if (*CurrentItem != NULL)
            result = EC_Normal;
        else
            result = EC_CorruptedData;
    }
    return result;
}

nbeck
Posts: 8
Joined: Fri, 2022-10-28, 12:56

Re: Invalid item in loop over sequences

#4 Post by nbeck »

Thank you for the suggestion. With this code everything works as expected! It also matches the implementation in other parts of DCMTK, e.g. DSRCodingSchemeIdentificationList::gotoNextItem() or DSRReferencedInstanceList::gotoNextItem(), so it is hopefully well tested.

You are probably aware of this, but just to avoid any confusion: This issue isn't specific to DRTROIContourSequence but affects most (or all) of the sequences in DCMRT.

In my humble opinion, the code would be much easier to maintain if these sequences were derived from a common templated class instead of duplicating the list code for each one. I tried to work on a patch for this, but since there are so many affected files which were generated by some script, I quickly gave up.

J. Riesmeier
DCMTK Developer
Posts: 2501
Joined: Tue, 2011-05-03, 14:38
Location: Oldenburg, Germany
Contact:

Re: Invalid item in loop over sequences

#5 Post by J. Riesmeier »

Thank you for the suggestion. With this code everything works as expected! It also matches the implementation in other parts of DCMTK, e.g. DSRCodingSchemeIdentificationList::gotoNextItem() or DSRReferencedInstanceList::gotoNextItem(), so it is hopefully well tested.
Thank you for your feedback. This is also what I was thinking. Don't ask me why the dcmrt implementation is different. At least the author was the same ;-)
You are probably aware of this, but just to avoid any confusion: This issue isn't specific to DRTROIContourSequence but affects most (or all) of the sequences in DCMRT.
You are right and, of course, I am also aware of this.
In my humble opinion, the code would be much easier to maintain if these sequences were derived from a common templated class instead of duplicating the list code for each one. I tried to work on a patch for this, but since there are so many affected files which were generated by some script, I quickly gave up.
That's true, but more than 90% of the dcmrt code (including the one of the sequences classes) is generated automatically from a machine-readable version of the DICOM standard, so it makes no difference. The dcmrt module was the result of a common research project with the German Cancer Research Center (DKFZ, Heidelberg).

nbeck
Posts: 8
Joined: Fri, 2022-10-28, 12:56

Re: Invalid item in loop over sequences

#6 Post by nbeck »

I understand. Thank you for your efforts, I am looking forward to the next release.

J. Riesmeier
DCMTK Developer
Posts: 2501
Joined: Tue, 2011-05-03, 14:38
Location: Oldenburg, Germany
Contact:

Re: Invalid item in loop over sequences

#7 Post by J. Riesmeier »

Thank you for your feedback. By the way, I also plan to update the automatically generated dcmrt classes to the latest edition of the DICOM standard, i.e. DICOM 2023a. I already started with this work, but, as always, there are still some issues: the DICOM RT people like to use the same Sequence Attributes with different content in either different Modules or even within the same Module!

J. Riesmeier
DCMTK Developer
Posts: 2501
Joined: Tue, 2011-05-03, 14:38
Location: Oldenburg, Germany
Contact:

Re: Invalid item in loop over sequences

#8 Post by J. Riesmeier »

The reported issue has been fixed in the current development version (see git repository). I've also updated the automatically generated classed to DICOM 2023b.

Post Reply

Who is online

Users browsing this forum: Ahrefs [Bot] and 1 guest