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

VentilationType Enum and VentilationControlMethods #99

Merged
merged 2 commits into from
Jun 20, 2019

Conversation

nllong
Copy link
Member

@nllong nllong commented Jun 20, 2019

see proposals/2019/Ventilation.md

@nllong nllong requested review from jasondegraw and markborkum June 20, 2019 15:08
Copy link
Contributor

@jasondegraw jasondegraw left a comment

Choose a reason for hiding this comment

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

I'll approve this PR, but I am not a fan of the proliferation of "None" as a method to indicate the lack of something when not including the element is more effective.

@jasondegraw jasondegraw merged commit 5bddc26 into develop Jun 20, 2019
@jasondegraw jasondegraw deleted the ventilatio-updates branch June 20, 2019 16:45
@markborkum
Copy link
Contributor

@jasondegraw In the context of BuildingSync enumerations, the terms "unknown/known" and "nothing/something" have distinct semantics when used in combination in assertions. There are three combinations:

  1. We don't know.
  2. We know that it is nothing.
  3. We know that it is something.

In Haskell, this could be modeled by an algebraic data type:

data Assertion a = Unknown | Known (Maybe a)

where

data Maybe a = Nothing | Just a

is from the standard library.

The absence of an XML element has a different semantics to the above: "no assertion."

@jasondegraw
Copy link
Contributor

@markborkum As I have previously pointed out, there are plenty of places where None has exactly no meaning in the schema. This exact context is not one of those cases, hence my approval.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants