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

Justkawal/complete new approach #186

Closed
wants to merge 149 commits into from

Conversation

justkawal
Copy link
Collaborator

@justkawal justkawal commented Jan 5, 2023

#Resolves Issue:

Description

  • A complete new configurable re-design
  • Easy to add functionality.
  • Well documented

Why?

  • More clean approach
  • Less boilerplate code
  • Short, clear and concised
  • Less dependency on other libraries

How?

  • By isolating the decode and encode functionality inside the basic type classes.
  • Lesser intermediate cyclic conversion of types: forth and back.

Testing Plan

  • Unit Test cases.
  • Large Unit Testing
  • Command: dart test

Comment on lines +93 to +103
output.pushByte(value.toInt() << 2);
} else if (value < 16384) {
output
..pushByte((value.toInt() & 0x3F) << 2 | 1)
..pushByte((value.toInt() >> 6).toUnsigned(8));
} else if (value < 1073741824) {
output
..pushByte((value.toInt() & 0x3F) << 2 | 2)
..pushByte((value.toInt() >> 6).toUnsigned(8))
..pushByte((value.toInt() >> 14).toUnsigned(8))
..pushByte((value.toInt() >> 22).toUnsigned(8));
Copy link
Collaborator

Choose a reason for hiding this comment

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

The value is already of type int here

      output.pushByte(value << 2);
    } else if (value < 16384) {
      output
        ..pushByte((value & 0x3F) << 2 | 1)
        ..pushByte((value >> 6).toUnsigned(8));
    } else if (value < 1073741824) {
      output
        ..pushByte((value & 0x3F) << 2 | 2)
        ..pushByte((value >> 6).toUnsigned(8))
        ..pushByte((value >> 14).toUnsigned(8))
        ..pushByte((value >> 22).toUnsigned(8));

final bytesNeeded = (value.bitLength + 7) >> 3;
output.pushByte(((bytesNeeded - 4) << 2).toUnsigned(8) | 3);
for (var i = 0; i < bytesNeeded; i++) {
output.pushByte(value.toUnsigned(8).toInt());
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
output.pushByte(value.toUnsigned(8).toInt());
output.pushByte(value.toUnsigned(8));

part of primitives;

class SimpleEnumCodec<A> with Codec<A?> {
final List<A?> list;
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should be an Map, because even simple enums can have sparse indexes, ex:

enum SomeEnum {
  #[index(0)]
  Value1,
  #[index(10)]
  Value2,
  #[index(30)]
  Value3,
}

}

class ComplexEnumCodec<V> with Codec<MapEntry<String, V?>> {
final Map<String, Codec<V?>?> map;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same case here, where do you store the enum index?

Comment on lines +57 to +61
output.pushByte(index);

final codec = map[value.key];

codec?.encodeTo(value.value, output);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Unnecessary line break.

Suggested change
output.pushByte(index);
final codec = map[value.key];
codec?.encodeTo(value.value, output);
output.pushByte(index);
final codec = map[value.key];
codec?.encodeTo(value.value, output);

import 'package:test/test.dart';

void main() {
group('I16 Decode Test:', () {
Copy link
Collaborator

Choose a reason for hiding this comment

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

We also need tests for OutOfBounds errors

import 'package:test/test.dart';

void main() {
group('I256 Decode Test:', () {
Copy link
Collaborator

Choose a reason for hiding this comment

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

We also need tests for OutOfBounds errors

import 'package:test/test.dart';

void main() {
group('I128 Decode Test:', () {
Copy link
Collaborator

Choose a reason for hiding this comment

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

We also need tests for OutOfBounds errors

Comment on lines +98 to +104
test(
'Given a 0x010100 it should be decoded to Option.some(Option.some(false))',
() {
final input = HexInput('0x010100');
final result = OptionCodec(OptionCodec(BoolCodec.instance)).decode(input);
expect(result, Option.some(Option.some(false)));
});
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nice

Comment on lines +5 to +11
group('SimpleEnumCodec Encode', () {
test('should encode and decode', () {
final output = HexOutput();
final codec = SimpleEnumCodec(['a', 'b', 'c']);
codec.encodeTo('b', output);
expect(output.toString(), '0x01');
});
Copy link
Collaborator

Choose a reason for hiding this comment

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

We need tests for sparse indexes, I think in this case we can do this:

SimpleEnumCodec.sparse({
  'a': 0,
  'b': 10,
  'c': 20,
});

Copy link
Collaborator

@Lohann Lohann Mar 2, 2023

Choose a reason for hiding this comment

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

Or the inverse (so we guarantee the index is unique):

SimpleEnumCodec.sparse({
  0: 'a',
  10: 'b',
  20: 'c',
});

Comment on lines +38 to +40
class Option extends Equatable {
final dynamic value;
final bool _isSome; // Workaround for Option<int?>.some(null)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
class Option extends Equatable {
final dynamic value;
final bool _isSome; // Workaround for Option<int?>.some(null)
class Option<T> extends Equatable {
final T value;
final bool _isSome; // Workaround for Option<int?>.some(null)

@justkawal justkawal closed this Mar 6, 2023
@Lohann Lohann deleted the justkawal/complete-new-approach branch March 16, 2023 17:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants