Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mergeFiles skips last entry #73

Closed
stefanhendriks opened this issue Feb 6, 2019 · 3 comments
Closed

mergeFiles skips last entry #73

stefanhendriks opened this issue Feb 6, 2019 · 3 comments

Comments

@stefanhendriks
Copy link

stefanhendriks commented Feb 6, 2019

I'm trying to use this implementation in my own software. I noticed that when passing 3 recordLines it ends up with 2 mergedLines. basically omitting the last line. This only is visible if you run it within a unit test (or you don't end with a -} recordLine).

Ie, this is a case where it happens (I made parseRecord public):

	@Test
	public void parseStatementLine() {
		Mt940Record mt940Record = Mt940Parser.parseRecord(list(
				"{1:F01KNABNL2HAXXX0000000000}{2:I940KNABNL2HXXXXN3020}{4:",
				":20:B9A02MSEE326KVMR",
				":61:1901011231C1,53N650NONREF//B8L31INTBB8ZCND1"  // this is skipped?
		));

		Assert.assertEquals(1, mt940Record.getEntries().size());
	}

I am aware this is not a valid MT940 record.

I am not proficient in the MT940 format, still figuring things out myself. Perhaps you could clarify a bit what the mergeLines intents to do? I already figured it merges indeed some lines not starting with : (which seems to be correct behaviour).

At this point it looks like it basically skips the last line. Ie there is no retVal.add(currentString.toString()); happening for the very last line, be cause of:

if (inMessage) {
				if (string.startsWith(":")) {
					retVal.add(currentString.toString());
					currentString = new StringBuilder();
				}
				currentString.append(string);
			} else ....

This adds the previous line, (in currentString), sets the new currentString, but it never gets added to retVal.

Adding:

		retVal.add(currentString.toString());

just before returning retVal in the mergeLines function seems to fix the test I describe above. I'm not sure if that is correct.

Just wanted to let you know I found this. For my implementation I need other fields. Perhaps I can offer some kind of PR when things are fixed on my end? I also try to translate any german verbs along the way.

@ccavanaugh
Copy link
Owner

I wouldn't say I'm the most proficient with MT940 either. The majority of the code was contributed by Arnout Engelen per the Copyright. I've done a few bits here and there to maintain it, but I've not dug very deep into the workings of it.

Improvements are always welcome.

Are you encountering invalid MT940 from online / banking sources?

@ccavanaugh
Copy link
Owner

Closing due to inactivity

@stefanhendriks
Copy link
Author

@ccavanaugh sorry, I haven't been able to spend time on this. I have a working MT940 implementation for our cases. I use integration tests to verify it works as expected. Although I did refactor a bit to make the code more testable. If you want - let me know - then I can send you some of my changes.

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

No branches or pull requests

2 participants