hapi-hl7v2 icon indicating copy to clipboard operation
hapi-hl7v2 copied to clipboard

DatumPath.numbersLessThan Bug? 🪲

Open milkshakeuk opened this issue 2 years ago • 3 comments

Hi @ohr, @jamesagnew

I have been looking to port some more hapi to nHapi and as I was porting DatumPath.numbersLessThan I started to write unit tests (which do not exist in hapi) and discovered that both the following are true but this shouldn't be the case should it?

How can both these statements be true?

[1, 1, 5, 5] < [1, 2]
[1, 2] < [1, 1, 5, 5]

milkshakeuk avatar Jul 05 '22 15:07 milkshakeuk

@ohr, @jamesagnew

I've done some more comparisons this time using IKVM to load the hapi-base-2.3.jar into my dotnet unit test project so I could compare the output and here are the results.

Looks like I haven't made a mistake in the porting process as they both return the same results give then example above.

image

image

milkshakeuk avatar Jul 07 '22 15:07 milkshakeuk

@jamesagnew @ohr

I believe this will fix it, I have made the change in the nhapi port and it seems to fix it (proven with unit tests), assuming that's the expected behaviour and I am not misunderstanding it.

public boolean numbersLessThan(DatumPath other)
{
	DatumPath extendedCopyThis = new DatumPath(this);
	extendedCopyThis.setSize(s_maxSize);

	DatumPath extendedCopyOther = new DatumPath(other);
	extendedCopyOther.setSize(s_maxSize);

	boolean lessThan = false;
	for(int i=1; !lessThan && (i<s_maxSize); ++i) {
		int this_i = ((Integer)extendedCopyThis.get(i));
		int other_i = ((Integer)extendedCopyOther.get(i));
		// this following line is the fix
		// if this_i is greater than other_i then just stop iterating as the answer is clearly false.
		if(this_i > other_i) break;
		lessThan = (this_i < other_i);
	}

	return lessThan;
}

I would create a PR however I have yet been successful in actually building hapi never mind running the test suite.

milkshakeuk avatar Jul 10 '22 13:07 milkshakeuk

@jamesagnew did you see this?

milkshakeuk avatar Oct 24 '23 20:10 milkshakeuk