Repository: intel/rohd
Pull request #71 (2021-12-23 06:30:00)

Feature/logicvalue_ofbool
## Description & Motivation

When helping another Intel Engineer using Rohd, I found the lack of conversion from bool to a LogicValue inconvenient, so I'm proposing adding this helper function to make LogicValues slightly more convenient.

## Related Issue(s)

None

## Testing

Added new test cases in logic-values_test.dart just for this feature.

## Backwards-compatibility

> Is this a breaking change that will not be backwards-compatible?  If yes, how so?

No breaking change

## Documentation

> Does the change require any updates to documentation?  If so, where?  Are they included?

Hopefully the API comment I added before the function translates properly into documentation there.  I'm not sure how to generate/test this locally.

mkorbel1: Thanks for your contribution!  There's actually an open issue for this as well: #34 

For consistency with other functions like `LogicValues.from`, `LogicValues.fromInt`, `LogicValues.fromBigInt`, etc. can I suggest naming this one `LogicValue.fromBool` instead of `ofBool`?

If you feel inclined to tackle the whole issue, a `fromBool` on `LogicValues` in addition to `LogicValue` would also be helpful!
eric-norige: I appreciate the push towards consistency, and maybe this is already resolved, but I notice that there's an existing tension between `.of()` and `.from()` in built-in classes like `List` and `Map` (https://stackoverflow.com/questions/50320220/in-dart-whats-the-difference-between-list-from-and-of-and-between-map-from-a).  This discussion suggests that if we want maximum consistency with the naming used in standard dart, all of these should be `.of()`, as there's no type conversion going on.  But I'll defer to your judgement on this for now, and the switch to `of` naming everywhere can be done independent of adding this code.

I also realize now that there's another style that this kind of function could be written, and that's `<bool>.toLogicValue()`, and that maybe this form would be better.  But that's a further different direction to go, and I'm not going to change to it now, just bringing it up as possibly a better way.

I will tackle `LogicValues.fromBool` as well, to produce a single-bit `LogicValues` with value based on that bool. It should be pretty easy. 


mkorbel1: I didn't know about this before, thanks for pointing it out.  I'm now convinced that all these `from`s should have really been `of`s.  I think it's fine to leave them as `of` or change them to `from` for now in this PR, and I'll open another issue to rename them all to `of` in the near future.  By the next release they should all be `of`.

The `<bool>.toLogicValue()` option is interesting.  I wonder if users would think to "look" for that utility as an extension of existing types rather than in the `LogicValue` class itself.  I was originally thinking of making `LogicValues` feel a bit like a `List` to use.

Thanks for taking on the `LogicValues` piece too!
mkorbel1: Filed #72 for `of` vs `from`

Patch:
diff --git a/lib/src/values/logic_value.dart b/lib/src/values/logic_value.dart
index 16239e83..644a37e1 100644
--- a/lib/src/values/logic_value.dart
+++ b/lib/src/values/logic_value.dart
@@ -24,6 +24,9 @@ class LogicValue {
   /// Logical value of `z`
   static const LogicValue z = LogicValue._(_LogicValueEnum.z);
 
+  /// Convert a bool to a one or zero
+  static LogicValue fromBool(bool v) => v ? one : zero;
+
   final _LogicValueEnum _value;
   const LogicValue._(this._value);
 
diff --git a/lib/src/values/logic_values.dart b/lib/src/values/logic_values.dart
index 5c81ab19..567647d5 100644
--- a/lib/src/values/logic_values.dart
+++ b/lib/src/values/logic_values.dart
@@ -557,6 +557,10 @@ abstract class LogicValues {
 
   const LogicValues._(this.length) : assert(length >= 0);
 
+  /// Converts `bool` [value] to a valid [LogicValues] with 1 bits either one or zero.
+  static LogicValues fromBool(bool value) =>
+      _SmallLogicValues(value ? 1 : 0, 0, 1);
+
   /// Converts `int` [value] to a valid [LogicValues] with [length] number of bits.
   ///
   /// [length] must be greater than or equal to 0.


Test patch:
diff --git a/test/logic_values_test.dart b/test/logic_values_test.dart
index 2196f227..c26dfc9e 100644
--- a/test/logic_values_test.dart
+++ b/test/logic_values_test.dart
@@ -28,4 +28,13 @@ void main() {
           equals(LogicValues.fromString('01xx' * 100)));
     });
   });
+
+  group('logic value', () {
+    test('fromBool', () {
+      expect(LogicValue.fromBool(true), equals(LogicValue.one));
+      expect(LogicValue.fromBool(false), equals(LogicValue.zero));
+      expect(LogicValues.fromBool(true), equals(LogicValues.fromString('1')));
+      expect(LogicValues.fromBool(false), equals(LogicValues.fromString('0')));
+    });
+  });
 }