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 Enums #299

Closed
luis-soares-sky opened this issue Feb 2, 2021 · 12 comments · Fixed by #484
Closed

Add Enums #299

luis-soares-sky opened this issue Feb 2, 2021 · 12 comments · Fixed by #484
Assignees
Labels
BrighterScript enhancement New feature or request New Language Feature A proposal for a new feature to be added to the BrighterScript language

Comments

@luis-soares-sky
Copy link
Contributor

luis-soares-sky commented Feb 2, 2021

This proposal is in progress. Feedback is welcome.


Basic declaration

By default, enum values are numeric and start from zero, with each subsequent key assuming the previous value + 1:

enum Direction
    Up
    Down
    Left
    Right
end enum

' Transpiles to...

function Direction() as Object
    return {
        Up: 0
        Down: 1
        Left: 2
        Right: 3
    }
end function

You can override any of the inferred values:

enum Direction
    Up = 1
    Down             ' = 2
    Left             ' = 3
    Right            ' = 4
end enum

You can also specify string values. However, all values will need to be initialized, and all of them need to be strings:

enum Direction       ' no need for "as String"
    Up = "up"
    Down = "down"
    Left             ' error: value needs an initialiser
    Right = 4        ' error: type mismatch
    Right = "right"  ' error: duplicate keys
end enum

Heterogeneous enums are not allowed in order to avoid "type mismatch" runtime errors.

Usage Examples

Value assignment and comparisons

Where possible, Brighterscript will make use of compile-time constants.

animDir = Direction.Right

if animDir = Direction.Left
    ' ...
end if

animElement(Direction.Down)

' Transpiles to direct value...
' NOTE: Type mismatch errors possible, but ideally the compiler would warn about these.

animDir = 3

if animDir = 2
    ' ...
end if

animElement(1)

As method argument and return type

function isGoingUp(currentAnimDir as Direction) as Boolean
    return currentAnimDir = Direction.Up
end function

function convertToDirection(someString as String) as Direction
    if someString = "up" then return Direction.Up
    if someString = "down" then return Direction.Down
    if someString = "left" then return Direction.Left
    if someString = "right" then return Direction.Right
    return -1
end function

' Transpiles to the enum's type...

function isGoingUp(currentAnimDir as Integer) as Boolean
    return currentAnimDir = 0
end function

function convertToDirection(someString as String) as Integer
    if someString = "up" then return 0
    if someString = "down" then return 1
    if someString = "left" then return 2
    if someString = "right" then return 3
    return -1
end function

Enum iteration

Sometimes it's useful to be able to iterate through all values:

for each value in Direction
    print value, value = Direction.Down
end for

' Transpiles to...

for each _bs_iter1 in Direction().Items(): value = _bs_iter1.value
    print value, value = 1
end for

' Output:
'  > 0         false
'  > 1         true
'  > 2         false
'  > 3         false
@luis-soares-sky luis-soares-sky changed the title enums Add Enums Feb 2, 2021
@elsassph
Copy link
Contributor

elsassph commented Feb 2, 2021

Pretty good.

I think iteration could be improved to use a "hidden" iterator and immediately getting the value. E.g. I think it would be valid:

for each _bs_iter in MyCustomEnum().Items(): value = _bs_iter.value
    print value, value = "Second"
end for

@TwitchBronBron
Copy link
Member

Thanks for the write-up! I had a few thoughts:

  1. enums should turn into compile-time constants. (i.e. MyEnum.MyValue becomes "MyValue"). In this way, we aren't cluttering m with lots of enum values that live in memory forever. In your current proposal, every scope's m would get its own copy of the enum object. There's not much value in keeping that enum around
  2. for loop iteration, we could still generate a function that just returns them.

@luis-soares-sky
Copy link
Contributor Author

@elsassph your suggestion makes a ton more sense and might be easier to implement, so I've included it in the proposal.

@TwitchBronBron regarding your first point: I currently work on an app where we use that MyEnum().Value pattern extensively. We won't be able to migrate it to brighterscript all at once due to its massive size, so my train of thought immediately went for maintaining .brs compatibility during that (currently unknown) transition period. However, what you're saying makes a ton of sense, so I've updated the proposal as you suggest.

@elsassph
Copy link
Contributor

elsassph commented Feb 3, 2021

For automatic string values you may want to include the enum name, e.g. MyEnum.Value would transpile to MyEnum__Value

@TwitchBronBron
Copy link
Member

I'm not overly fons of that idea. You shouldn't normally be mixing and matching different enums. We can catch that kind of thing during validation. Typescript doesn't do it this way either if I remember correctly.

@TwitchBronBron
Copy link
Member

@TwitchBronBron regarding your first point: I currently work on an app where we use that MyEnum().Value pattern extensively. We won't be able to migrate it to brighterscript all at once due to its massive size, so my train of thought immediately went for maintaining .brs compatibility during that (currently unknown) transition period. However, what you're saying makes a ton of sense, so I've updated the proposal as you suggest.

If you stick with my proposal, you could easily write a bsc plug-in to auto generate those functions to hold you over until you get totally converted to bsc.

@luis-soares-sky
Copy link
Contributor Author

If you stick with my proposal, you could easily write a bsc plug-in to auto generate those functions to hold you over until you get totally converted to bsc.

I was thinking about them while modifying the proposal, but I didn't know if that could be done given my inexperience with bsc. That gives a bit more confidence, thank you.

For automatic string values you may want to include the enum name, e.g. MyEnum.Value would transpile to MyEnum__Value

I'm not overly fons of that idea. You shouldn't normally be mixing and matching different enums. We can catch that kind of thing during validation. Typescript doesn't do it this way either if I remember correctly.

This actually makes me pivot and wonder if we should align the proposal to have enums be numeric by default, instead of auto-generated strings. It would bring the feature more in line with what Typescript, C#, and other languages already do:

enum NumericDirection
    Up         ' 0
    Down       ' 1
    Left       ' 2
    Right      ' 3
end enum

enum NonZeroNumericDirection
    Up = 1     ' 1
    Down       ' 2
    Left       ' 3
    Right = 3  ' 3  (allows duplicate values)
    Front      ' 4  (assumes "previous value + 1")
end enum

enum StringDirection as String
    Up = "up"
    Down = "down"
    Left = "left"
    Right = "right"
    Front = "right"  ' (valid: allows duplicate values)
    Front = "front"  ' (error: duplicate keys)
    Bottom           ' (error: initialiser needs to be specified when enum is not numeric)
end enum

@TwitchBronBron
Copy link
Member

I've been using typescript as a model/inspiration for many things in bsc, since BrightScript shares many similarities with JavaScript. I like this idea of aligning with their implementation of enums.

@elsassph
Copy link
Contributor

elsassph commented Feb 4, 2021

Agreed that ultimately using int by default should have better performance.

@luis-soares-sky
Copy link
Contributor Author

@TwitchBronBron @elsassph proposal updated.

@TwitchBronBron
Copy link
Member

TwitchBronBron commented Feb 4, 2021

This looks great!

Another thought: we don't need the as SomeType syntax when declaring an enum. The compiler can derive the enum data type based on its contents (probably by looking at the type of the first enum member).

Do we see any value in supporting types other than int and string? Like would we expand this to support any primitive type (float, double, bool)? I'm leaning towards constraining it to just int and string, but would be interested to hear other opinions on the matter.

@luis-soares-sky
Copy link
Contributor Author

Another thought: we don't need the as SomeType syntax when declaring an enum. The compiler can derive the enum data type based on its contents (probably by looking at the type of the first enum member).

Do we see any value in supporting types other than int and string? Like would we expand this to support any primitive type (float, double, bool)? I'm leaning towards constraining it to just int and string, but would be interested to hear other opinions on the matter.

FWIW from the perspective of a developer that intends to make heavy use of this feature, and for a first iteration, I don't see any problems in restricting enums to int and string, and deriving the type from the first content.

@TwitchBronBron TwitchBronBron added BrighterScript New Language Feature A proposal for a new feature to be added to the BrighterScript language enhancement New feature or request labels Feb 11, 2021
@TwitchBronBron TwitchBronBron self-assigned this Jan 13, 2022
@TwitchBronBron TwitchBronBron added this to Backlog in New Language Features via automation Jan 13, 2022
@TwitchBronBron TwitchBronBron moved this from Backlog to In Progress in New Language Features Jan 13, 2022
New Language Features automation moved this from In Progress to Done Feb 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BrighterScript enhancement New feature or request New Language Feature A proposal for a new feature to be added to the BrighterScript language
Projects
Development

Successfully merging a pull request may close this issue.

3 participants