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

Add mixins to help with constants and symbols #194

Merged
merged 2 commits into from
Nov 20, 2023

Conversation

chiphogg
Copy link
Contributor

@chiphogg chiphogg commented Nov 19, 2023

Constants (#90) and symbols (#43) have a lot of similarities. For
example:

  • Multiplying by a raw number produces a quantity.
  • Multiplying by a quantity changes the units of the quantity.
  • They can compose with other instances of the same family (e.g., the
    product of two constants is a constant).

Let's use the term "wrapper" to refer to constants and symbols here. To
make each wrapper as easy as possible to implement, I've created some
"mixin" classes, each of which adds the full set of multiplication and
division operations for a single combination of the wrapper and some
other family of types (raw numbers, quantities, other wrappers, etc.).

These wrappers use a "CRTP-ish" syntax. But instead of the type
itself
being the first template parameter (e.g., Wrapper<Unit>), we
provide the wrapper and unit as two separate parameters (i.e.,
Wrapper and Unit). The reason is that we need to know the Unit,
and providing it explicitly makes it easy to get. These first two
parameters are the same for all mixins, which makes the list-of-mixins
at the class definition easier to read.

The unit tests are based on an example, UnitWrapper, which aggregates
a variety of properties. The forthcoming "true" classes, Constant and
SymbolFor, will be defined very similarly to this.

Finally, a word about the plan for "numeric" inputs. For now, we are
restricting to std::is_arithmetic. This doesn't mean we don't support
other reps; it just means we won't be able to create quantities from
them via constants or symbols for a while. The evolution plan is to
create a well-defined concept that defines what is a valid rep, and then
replace std::arithmetic with that concept (see also #52).

Test plan:

  • Add extensive new unit tests
  • Manually uncomment each individual "uncomment to test" case

Constants (#90) and symbols (#43) have a lot of similarities.  For
example:

- Multiplying by a raw number produces a quantity.
- Multiplying by a quantity changes the units of the quantity.
- They can compose with other instances of the same family (e.g., the
  product of two constants is a constant).

Let's use the term "wrapper" to refer to constants and symbols here. To
make each wrapper as easy as possible to implement, I've created some
"mixin" classes, each of which adds the full set of multiplication and
division operations for a single combination of the wrapper and some
other family of types (raw numbers, quantities, other wrappers, etc.).

These wrappers use a "CRTP-ish" syntax.  But instead of the _type
itself_ being the first template parameter (e.g., `Wrapper<Unit>`), we
provide the _wrapper_ and _unit_ as two separate parameters (i.e.,
`Wrapper` and `Unit`).  The reason is that we need to know the `Unit`,
and providing it explicitly makes it easy to get.  These first two
parameters are the same for all mixins, which makes the list-of-mixins
at the class definition easier to read.

The unit tests are based on an example, `UnitWrapper`, which aggregates
a variety of properties.  The forthcoming "true" classes, `Constant` and
`SymbolFor`, will be defined very similarly to this.

Test plan:

- [x] Add extensive new unit tests
- [x] Manually uncomment each individual "uncomment to test" case
@chiphogg chiphogg added the release notes: ✨ lib (enhancement) PR enhancing the library code label Nov 19, 2023
@chiphogg chiphogg requested a review from geoffviola November 19, 2023 21:47
Comment on lines +30 to +36
struct UnitWrapper : MakesQuantityFromNumber<UnitWrapper, Unit>,
ScalesQuantity<UnitWrapper, Unit>,
ComposesWith<UnitWrapper, Unit, UnitWrapper, UnitWrapper>,
ComposesWith<UnitWrapper, Unit, QuantityMaker, QuantityMaker>,
ForbidsComposingWith<UnitWrapper, Unit, QuantityPointMaker>,
ForbidsComposingWith<UnitWrapper, Unit, QuantityPoint>,
CanScaleByMagnitude<UnitWrapper, Unit> {};
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note to reviewers: this is probably the best starting point. The goal of this PR is to enable us to write something like this. The test cases below demonstrate what capabilities this kind of definition provides. Then, with that motivation, you can check out the implementations in the previous file.

Copy link
Contributor

@geoffviola geoffviola left a comment

Choose a reason for hiding this comment

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

Makes sense. The concept-like implementation is interesting. The tests look good.

@chiphogg chiphogg merged commit c20bf4e into main Nov 20, 2023
@chiphogg chiphogg deleted the chiphogg/wrapper-ops#43#90 branch November 20, 2023 23:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release notes: ✨ lib (enhancement) PR enhancing the library code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants