bmx-ng / brl.mod

BlitzMax Runtime Libraries, for BlitzMax NG.
12 stars 11 forks source link

new TSomething versus TSomething.Create() #115

Open GWRon opened 5 years ago

GWRon commented 5 years ago

Just recognized that I got pretty much used to @woollybah (and others) non-procedural way of creating type instances.

I always did: list = CreateList() but now am used to list = new TList. So far no problem isn't it?

Now guess what happens for a mutex or thread? You will run into issues as their Create:TThisType() functions do something more. Especially such types which just have a "handle" could easily have their "Create-stuff" done in New().

Or is there speaking something against such a change? Only exception is if you do not want to "initialize" such a type - but most often this isn't desired.

For now I feel unsure if new TThisType is a good idea as you cannot assure to always get what you expect (think of unseen changes to a module).

HurryStarfish commented 5 years ago

Or is there speaking something against such a change?

Not really. In legacy, New couldn't have parameters, so Create functions/methods were commonly used as a replacement. But That's no longer necessary in NG. The only other reason I can think of would be if you're expecting the object creation to frequently fail - in that case, returning and checking for Null would be slightly faster than throwing and catching an exception. That isn't normally the case though, and even if so, New should either still produce a valid object, or be disabled (by making it private) to prevent accidental misuse.

HurryStarfish commented 5 years ago

Only exception is if you do not want to "initialize" such a type - but most often this isn't desired.

In that case, providing an Initialize/Init method would probably be the way to go. We might also want to provide something like this in the standard library (once we have fully working generics and closures).

DivineDominion commented 5 years ago

Interesting! I didn't know that Legacy Bmax didn't support parameterized New and found the factory pattern via Create to be oddly unnecessary in today's code.

I think we should update the docs to state more clearly what's supposed to be the way to go. I think parameterized constructors are the best candidate as they are a built-in mechanism. If you folks agree, I'd happily whip up a docs PR.

GWRon commented 5 years ago

So would it be a good idea to "migrate" code to the new possibilities?

so

    Rem
    bbdoc: Create a new thread
    End Rem
    Function Create:TThread( entry:Object( data:Object),data:Object )
        Local thread:TThread=New TThread
        thread._entry=entry
        thread._data=data
        thread._running=True
        thread._handle=threads_CreateThread( _EntryStub,thread )
        Return thread
    End Function

would become

    Method New( entry:Object(data:Object), data:Object )
        self._entry=entry
        self._data=data
        self._running=True
        self._handle=threads_CreateThread( _EntryStub,thread )
    End Method

    'deprecated, kept for now to stay backwards compatible
    Rem
    bbdoc: Create a new thread
    End Rem
    Function Create:TThread( entry:Object( data:Object),data:Object )
        Return New TThread(entry, data)
    End Function

With the overloading capabilities of NG this means you could run New TThread without params (so same behaviour as now) or New TThread(func, data) to have the initialization effect Create() offers at the moment.

For this particular example we would have the problem that a New TThread will result in something "non working". _running will never become true etc. So maybe in this case we should provide a Method New() which throws an error stating on how to properly create a new TThread. If you really needed a new TThread without initialization then a Method New(barebone:int) could do.