Custom record extractor implementation for VBVR data files, should it not use rawRecordExtractor?

[mark-weghorst]

Background [Optional]

In #338 @yruslan added support for a custom record extractor interface so that users could write their own custom record extractors. I have implemented a custom record extractor to read variable block, variable record datasets as defined by the IBM documentation I referenced in #338

In implementing this record extractor my first attempt resulted in a bug where the last block would only return a single record.

I believe that this is because when I call RawRecordContext.next(n) to read the last block of records it advances the "pointer" to EOF, which makes hasNext() return false even if we haven't processed all of the records from that block.

My second attempt which I've attached, attempts to "decouple" hasNext() from the RawRecordContext by implementing a buffering queue and the logic to read blocks inside of hasNext() which I don't regard as an ideal solution.

The thing I don't like about my current implementation is that it has logic inside of hasNext() and generally seems in-elegant and complicated.

Question

Should support for VBVR record types be implemented as a separate block-aware reader type in Cobrix? If not, do you see a better and more elegant way than what I have implemented?

VariableBlockRecordExtractor.scala.zip

[yruslan]

Thanks for the contribution! I'll take a look and will come back to you

[yruslan]

I've checked the implementation and it looks good. It would be really nice to have the VBVR feature as part of Cobrix. I have a couple of ideas on how it can be improved a little, but I'd try them out once a test suite is added for the implementation.
If you are okay with using the implementation as a custom record extractor for now, I can integrate this as a native Cobrix feature. I can add tests as well. But I can do it only in September.

I like to ask you to add the file to the project retaining you as the original author, and then I can start working and improving it.

Specifically, the steps are:

• Fork Cobrix
• Create a branch
• Add the file to za.co.absa.cobrix.cobol.reader.extractors.raw package (cobol-parser module)
• Add the copyright header tp the top of the file by copying one from any other Scala files in Cobrix
• Create a pull request to Cobrix

After that, I can merge the pull request into a separate branch, add tests, possibly improve the code, and then create a pull request with you as a reviewer for you to check if you agree with the changes. And then, the feature will be available in the next release of Cobrix.

Let me know if it works for you.

[mark-weghorst]

Thank you this is wonderful news indeed, it may take me a few days since I have some other higher priority work I need to get to first, but I've added a story to my board to prepare this PR for you.

[mark-weghorst]

The PR is done, here are a few things that came to mind and I'm sure you are already thinking about:

• The current implementation only supports big endian block and record descriptor words
• I'm not sure what your plans are for configuration switches, but as written this will only work if specified as a custom record extractor

[yruslan]

Yes, all above considerations are something that will be addressed.
I'll merge the PR and going to start working on the feature next week.

• Both big endian and little endian will be supported
• The feature will be built-in