SourcePointUSA / sp-roku-sdk

SP SDK for ROKU.
2 stars 0 forks source link

Strange use of instance vars in SDK #6

Closed g4r37h closed 2 years ago

g4r37h commented 3 years ago

Hi guys

There's some odd use of instance vars in the SDK, whereby firstly values are declared and initialised separately, and then some select instance vars are added to another instance var that is later added to the global scope, i.e:

m.accountId = invalid
m.propertyHref = invalid
m.legislationsEnabled = invalid

m.accountId = accountId
m.propertyHref = propertyHref
m.legislationsEnabled = legislationsEnabled

It's perfectly fine to both declare and initialise a variable in a single line, so the first block there isn't required.

But more importantly, there is also seems odd behaviour with regarards to instance vars that are defined for each of these values individually, then they're added to another instance var object that serves as some kind of wrapper, and then this wrapper instance is itself set on the global scope. That means you have at least 3 references to the same values - two in the SDK's scope and one on the global scope - and that's just from within the SDK class without even looking at the other classes that are possibly further duplicating references.

And given that some of the values are value types rather than reference types, I think that increases the probability of bugs within the codebase as you have several potential sources of truth.

taraschi-sp commented 2 years ago

Hi @g4r37h ,

We use a community project called BrighterScript - https://github.com/rokucommunity/brighterscript - which is a superset of Brightscript and compiles down to it.

As a result some of the compiled code may look strange.