iso8583 icon indicating copy to clipboard operation
iso8583 copied to clipboard

Published 20 hours ago • verified publisher icon moov-io

auto bitmaps size

Open mfdeveloper508 opened this issue 3 years ago • 3 comments

  • remove maxBitmaps
  • use auto size according to specification
  • added over range check
  • updated test codes

mfdeveloper508 avatar Jul 01 '22 12:07 mfdeveloper508

This LGTM but I'd like @alovak to review it when he's back.

adamdecaf avatar Jul 01 '22 18:07 adamdecaf

Hey, @alovak can you take a look at this?

wadearnold avatar Aug 04 '22 17:08 wadearnold

Sorry for the delay. I hope to review and merge it this/next week @mfdeveloper508 @wadearnold

alovak avatar Sep 26 '22 18:09 alovak

Can we get this merged @alovak ?

wadearnold avatar Nov 03 '22 22:11 wadearnold

The main issue with this implementation is that we should manually set the bitmap size, while in some cases we may not know it. The current implementation automatically reads the next bitmap if the first bit of the current bitmap is set and the solution suggested in the PR breaks this feature.

We should not do/have things like bitmap.SetMapSize(2) here as we don't actually know the length of the bitmap. The bitmap should automatically read two bitmaps.

The alternative version is here: https://github.com/moov-io/iso8583/pull/211

alovak avatar Jan 31 '23 15:01 alovak