search

Ephenodrom / Dart-Basic-Utils

A dart package for many helper methods fitting common situations
MIT License
364 stars 77 forks source link

hexToInt and intToHex #29

Closed awhitford closed 4 years ago

awhitford commented 4 years ago

hexToInt

https://github.com/Ephenodrom/Dart-Basic-Utils/blob/3b53270fc71b9d94386a0a5d7240c4a59b80665e/lib/src/ColorUtils.dart#L13-L26

What do you think should happen with these test cases?

    expect(ColorUtils.hexToInt('000001'), 0xFF000001);
    expect(ColorUtils.hexToInt('00000002'), 0x00000002);
    expect(ColorUtils.hexToInt('3'), 0x00000003);
    expect(ColorUtils.hexToInt('#4'), 0xFF000004);

The last one fails -- it returns 0xFF4 instead of 0xFF000004.

The code seems to be unclear about its assumptions. If a Hex Color Code should always be 6 or 8 characters, then the function should either assert that, or check that.

I'm guessing that you supply an FF opacity when no alpha is provided. That's fine, but then presumably any value less than 6 digits should be treated as having preceding zeros.

intToHex

https://github.com/Ephenodrom/Dart-Basic-Utils/blob/3b53270fc71b9d94386a0a5d7240c4a59b80665e/lib/src/ColorUtils.dart#L28-L38

What do you think should happen with these test cases?

    expect(ColorUtils.intToHex(0xFF0000FA), '#0000FA');
    expect(ColorUtils.intToHex(0x100000FB), '#0000FB');
    expect(ColorUtils.intToHex(0x000000FC), '#0000FC');

The last one fails -- it returns "#FC" instead of "#0000FC".

The intToHex function is stripping the alpha/opacity, so a AARRGGBB hex code will turn into #RRGGBB.

Generally, one would assume thati == hexToInt(intToHex(i)), but if there is a non FF alpha value, then that is not true.

Ephenodrom commented 4 years ago

@awhitford The PR is closed and every unit test looks good. Your improvements a legit and I think we can use it like this. Will release a new version with this and the other fixes next week.