Skip to content

Finom engine#95

Open
BPere wants to merge 1 commit into
fruitl00p:masterfrom
BPere:finom-engine
Open

Finom engine#95
BPere wants to merge 1 commit into
fruitl00p:masterfrom
BPere:finom-engine

Conversation

@BPere
Copy link
Copy Markdown

@BPere BPere commented Mar 26, 2026

Engine parser for the FINOM MT940 file format

Copy link
Copy Markdown
Owner

@fruitl00p fruitl00p left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the added engine. I do have some comments before it can be merged in though.

namespace Kingsquare\Parser\Banking\Mt940\Engine;

use Kingsquare\Banking\Statement;
use Kingsquare\Banking\Hsbc\HsbcTransaction;
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like this doesnt belong here?

$data = $this->getCurrentStatementData();
if (preg_match('/:25:FNOM([^:\r\n]+)/', $data, $results) && $results[1] !== '') {
$account = trim(preg_replace('/EUR\s*$/i', '', trim($results[1])));
return trim($account);
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

its already trimmed :)

}

/**
* Overloaded: Is applicable if first line starts with :20:AI.
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the comment doesn't match the actual code

$this->engine->loadString(file_get_contents(__DIR__ . '/sample'));
}

public function testParseStatementBank()
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd really like to see a full test suite covering all the various overloaded methods to ensure the correct parsing of the statements themselves.

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.

2 participants