wurstscript / WurstStdlib2

WurstScript Standard Library Version 2
Apache License 2.0
55 stars 53 forks source link

Function module #405

Closed Cokemonkey11 closed 2 years ago

jlfarris91 commented 3 years ago

I like the idea of reducing the number of bespoke interfaces/abstract classes and I did the same in my library.

My 2 cents is that I'm not a fan of these names, but I'm also biased as a primarily .net developer. I'd suggest keeping it simple and to common terminology for ease of adoption. An Action returns nothing and can have parameters; A Function returns something and can optionally have parameters.

Action (takes nothing returns nothing)
Action1<TArg1> (takes 1 arg returns nothing)
Action2<TArg1, TArg2>
etc.

Func<TRet>
Func1<TArg1, TRet>
Func2<TArg1, TArg2, TRet>
etc.

These are much more readable and easy-to-understand for me, personally.

Another common issue with these function objects is that they are often passed as parameters to some other function to be invoked and that invoking function doesn't have the context to know whether or not to destroy the object after the function object is invoked. Most of the calls in wurststdlib make the assumption that the object is a one-time-use but in many cases I don't want to keep destroying and recreating these objects unnecessarily. In my library I've added a m_destroyAfterUse that gives the caller that context. I'd recommend the same here.

Here's my implementation in case anyone is interested. You can ignore the multicast stuff, that's probably overkill for wurststdlib. I'll also probably get rid of the get/setDestroyAfterUse methods and just make the field public to avoid more bloat.

https://github.com/jlfarris91/WurstCore/blob/master/wurst/Action.wurst https://github.com/jlfarris91/WurstCore/blob/master/wurst/Func.wurst

Frotty commented 3 years ago

I like the idea of reducing the number of bespoke interfaces/abstract classes and I did the same in my library.

I dislike it. I think it's best practice to have specific and strictly typed interfaces instead of meaningless "Functions". Especially in Wurst it's bad because it creates huge dispatch overhead and shares index spaces.

Here's my implementation in case anyone is interested

If you want these functional interfaces, simply use wurst-lodash it also comes with auto destroy functionality. Hence I would say this isn't stdlib material. Any reason you and coke aren't using it?

Btw what's with the weird comment lines in your code?

Cokemonkey11 commented 3 years ago

@Frotty

I think it's best practice to have specific and strictly typed interfaces instead of meaningless "Functions".

Not going to fight this battle, but my take is that i write the game interface for "Consumer" or "Predicate" almost every time i wrote wurst. Often also need to find a reference to remember that correct syntax of functional interface. This would help with that imo. Happy to close if you disagree still

@jlfarris91 thanks for commenting. These terms come from Java -- not invented here

jlfarris91 commented 3 years ago

Especially in Wurst it's bad because it creates huge dispatch overhead and shares index spaces.

@Frotty Would you mind explaining this a bit more? Based on just my assumptions about how Wurst works behind the scenes, and I'm probably very wrong, I would think that a bunch of 1-to-1 bespoke interface-to-implementations would cause the same (or worse) dispatch overhead and shared index space problems.

If you want these functional interfaces, simply use wurst-lodash it also comes with auto destroy functionality. Hence I would say this isn't stdlib material. Any reason you and coke aren't using it?

Didn't know it existed tbh. At this point I've written everything I need myself (including function interfaces, obviously) so it doesn't make much sense to adopt it now (at least for my current project). The WurstCore library is a superset of wurst-lodash and an extension of wurststdlib2. I have a ton of useful utilities in there take a look sometime. I could probably break it up into a bunch of smaller libraries but library dependency issues discouraged me from continuing down that path.

Btw what's with the weird comment lines in your code?

It makes it easy to quickly identify major scopes of code at a glance. I put them above any package class, module, function and enum declarations. It's a personal standard practice that I picked up in the industry; I find it valuable. I also do the same for function declarations at the class level but use hyphens instead of equal signs.

These terms come from Java -- not invented here

@Cokemonkey11 Ah gotcha. Gosling's fault then ;)

Frotty commented 2 years ago

@Frotty Would you mind explaining this a bit more? Based on just my assumptions about how Wurst works behind the scenes, and I'm probably very wrong, I would think that a bunch of 1-to-1 bespoke interface-to-implementations would cause the same (or worse) dispatch overhead and shared index space problems.

All closures of type Function would share the same index space. If you define domain specific interfaces those have their own id space. Every time a closure is invoked it needs to be dispatched, doing a binary search in all type ids.

Cokemonkey11 commented 2 years ago

Having given this some further thought, I think it only makes sense to merge if there's a path forward to getting internal usage of this package in the stdlib

Given that we can't do that, it doesn't seem fair to recommend this in the stdlib

I do think this points to a bit of a flaw though - we should be able to do this without perf impact. I consider this cleaner than the current status quo