Class specification

[TwitchBronBron]

This is a proposal for classes. Here is the brighterscript syntax:

Syntax

class VideoPlayer
    'public constructor
    public sub New(url as string)
        m.url = url
    end sub

    'properties
    public url as string
    private isPlaying as boolean = false

    'public method
    public play() 
        m.isPlaying = true
        m.updateVideoStatus()
    end function

    'private method
    private updateVideoStatus()
        'do something private here
    end sub

end class

Output

function VideoPlayer(url as string)
    instance = {
        url: nothing,
        'underscore denotes private
        _isPlaying: nothing,
        play: VideoPlayer_play,
        _updateVideoStatus: VideoPlayer__updateVideoStatus
    }

    'constructor body
    instance.firstName = firstName
    instance.lastName = lastName
    instance.fullName = firstName + " " + lastName

    return instance
end function

function VideoPlayer_play()
    m.isPlaying = true
    m._updateVideoStatus()
end function

'double underscore (one for namespacing, the other for indicating private

function VideoPlayer__updateVideoStatus()
    'do something private here
end sub

Usage

BrighterScript

'usage
sub main()
    player = new VideoPlayer("some_url")
    player.play()
end sub

BrightScript

sub main()
    player = VideoPlayer("some_url")
    player.play()
end sub

[georgejecook]

can you please demosntrate where the constructor (i.e. New) code goes to? Also how does namespacing affect this?

[georgejecook]

also - can we get the className, or some other identifying type, for class comparison purposes?

[TwitchBronBron]

There already a comment in the output showing you where the constructor body goes.

And the class name is a great idea!

[georgejecook]

ah - I missed it, as I was expecting to see the m.url = url

aaaannndddd - we should be able to extend, even if it's just brute force overrides.

[chrisdp]

Is there anyway we could do classes ether as AA out put or as Node output. I say this because internally we avoid AA's as much as possible.

[georgejecook]

this is a good suggestion - so something like NodeClass? whereby

• automatcially creates a className.xml,
• imports the declaring brs file into className.xml
• the public methods become <function> interface entries in className.xml
• public fields become <field> entries on className.xml

There's a lot in this idea - I think you should raise it as a separate issue - as it's a hugely useful; but also huge feature; but (imho) way beyond the scope of pobo (i.e. aa) classes

[chrisdp]

^ New issue opened to track the ability so have both Node or AA based classes.

[sky-celsofernandes]

@TwitchBronBron you can compile to something similar to this, it's more similar to source code but won't work in components, right? :(

function VideoPlayer(url as string)
    instance = {
        url: nothing
        'underscore denotes private
        _isPlaying: nothing

        New: function()
          m.url = url
        end function

        play: function VideoPlayer_play()
            m.isPlaying = true
            m._updateVideoStatus()
        end function

        'double underscore (one for namespacing, the other for indicating private
        _updateVideoStatus: function VideoPlayer__updateVideoStatus()
            'do something private here
        end sub
    }

    instance.New()

    return instance
end function

[TwitchBronBron]

@georgejcook you had some concerns about the function-on-object approach. Could you explain them here?

[georgejecook]

not especially opposed to it; but I have 3 things to consider, which make me wonder if it's the right first version of the implementation:

1. Are there any performance/memory optimization differences for inline functions?
2. Doesn't this then necessitate a source map feature to enable debugging? Does that mean IDE debugging will delay this feature? If so, I prefer leaving the code to match line numbers as much as posbbile - I currently do this in my preprocessor, so that users's of it can still breakpoint debug. I understand source maps is a feature; but this can be shipped quickly, without sourcemap support with the non-nested function style
3. Does this have implications for non brighterscript users picking up our brighterscript transpiled libraries? will they be able to find definitions of these functions and navigate to them comfortably?

Not offering guidance, these are questions.

[TwitchBronBron]

@georgejecook wasn't there another concern about the function-on-object approach? Something about function call tracking?

[georgejecook]

is this an error?

    public play() 
        m.isPlaying = true
        m.updateVideoStatus()
    end function```

should it be `public function play()`?

[georgejecook]

Also - what's the value of the double underscore for private functions for their function name? e.g. VideoPlayer__updateVideoStatus

I'm not a fan of this - the underscore on the member name codifies the visibility on the class field, which seems correct to me; but I'm not sure what the intent/value is of manipulating the source function.

To my mind, the source function should remain unchanged, beyond any namespace manipulations, as the class is merely aggregating functions together into a conceptual collection of members - I don't think it has the remit to change the source function name.

[georgejecook]

we never discussed extending classes I've implemented this, in a crude fashion (no surprise there); but I don't know what to do about constructors.. I was going to go with requiring a m.super(args), in the new method

this is all well and good; but we'd need to wrap all object instantiation in a function to drive the logic - as it would need to 1. fully create the object prior to calling constructor, 2. call constructor

snip! - silly idea

so, in my mind, we would compile something like foo = new VideoPlayerVM(video), to foo = BSRT_createClass(VideoPlayerVM, [video, parent])

that way the VideoPlayers' constructor, could be called once all of the various aa fields/functions are mixed in from all parents, and the super calls would chain down, with a fully constructed class I'm going to implement it like this for now, as I need extended classes, and have supported them in my own code for quite some time but it get's at the need, again to have a runtime library in every scope that instantiates a class - I also do something similar in maestro-cli, where I know if a scope contains xml bindings, and if so, it auto-imports the binding framework into the scope

I think this kind of thing makes a solid argument for separting out the constructor. This is what I do in my implementation, and it is working great, with extended classes.

[georgejecook]

Actually, I think I've got a nice clean solution for this. I separate out the notions of building and constructing.

Here's some brs that I'm compiling to demonstrate

The external constructor is generated as such, and if the class is extended, then the base instanc is it's builder (which cascades, until eventually being {}, when there are no more super classes)

      function ZUIBContentVM(riversJson, subclass = invalid)
        instance = _ZUIBContentVM_builder$()
        instance._ZUIBContentVM$(riversJson)
        return instance
      end function

The second line makes the call to the constructor (Designated new method).

We need only then change the m.super(foo,bar) call, into the name of the super classes constructor, so in ZUIBContentVM.new, it compiles like this:

 function  ZUIBCVM2_new(riversJson, subclass = invalid)
          m._BaseViewModelTest$(riversJson, "this is cool")
          if riversJson.component_type <> invalid
            componentName = riversJson.component_type

And the builder that is generated is like this:

   function _ZUIBContentVM_builder$()
      instance = _BaseViewModelTest_builder$()
      ZUIBContentVM_instance = {

         _ZUIBContentVM$: ZUIBCVM2_new

        load: ZUIBCVM2_load
        cancelLoad: ZUIBCVM2_cancelLoad
        destroy: ZUIBCVM2_destroy
        focusOnContentAtIndex: ZUIBCVM2_focusOnContentAtIndex
        onKeyPressUp: ZUIBCVM2_onKeyPressUp
        fetchFeed: ZUIBCVM2_fetchFeed

        content: []
        feed: invalid
        selection: invalid
        keyPressAction: "none"
        isShowingLoadingSpinner: false
        originEntry: invalid
        isLoadingFeedChildren: false

        _contentMap: {}

        _onFeedResult: ZUIBCVM2_onFeedResult
        _isResultFromRequest: ZUIBCVM2_isResultFromRequest
        _getConnectedTarget: ZUIBCVM2_getConnectedTarget
      }
      instance.append(ZUIBContentVM_instance)
      return instance
      end function

oh - and here's the source of the class to make it easier to see what's happening.. https://github.com/georgejecook/maestro-cli/blob/beta/src/test/stubProject/components/screens/tempTest/ZUIBContentVM.bs

[georgejecook]

been thinking about the class output retuning _privateMethod.. thinking about this, isn't that a bit imposing?

The IDE will know what is public or private, so that info isn't needed there. If we were to ever impose this, somehow, someway at runtime, we could simply add introspection.

[TwitchBronBron]

I have started working on this.

[georgejecook]

awesome.. in case you're interested - I've written maestro framework in brighterscript, and my client project is also in brighterscript (though I can't share those sources)

I've followed your spec; but added extend, and super keyword for constructors, as this very quickly became an apparent oversight, and needed implementing.

Here's an example of a typical class

https://github.com/georgejecook/maestro/blob/master/framework/src/source/maestro/view/viewModel/BaseViewModel.bs

fwiw, I'll be happy to guinea pig your compiler, during dev, should you like. Rooibos and maestro are both brighterscript projects, with import namespace and class, so you could use them as well, if you want some real projects to run against.

[georgejecook]

He @TwitchBronBron , more feedback following use over at maestro..

I think we should identify class name invocations without new keyword, and flag them.

3 reasons:

1. spot accidental usage
2. new keyword adds vital context
3. in future we might do further manipulations for class instantiatioon, which will get missed by not using the new keyword

[TwitchBronBron]

I agree!

[TwitchBronBron]

This has been completed.