Store generic Func T inside a colletion [Answered]

SStay8/20/2022
I have a generic Func<T>, which i wan't to store in a Dictionary, but i don't know how to do that since apparently the restriction of T is not enough
SStay8/20/2022
I have this method which takes a Func<Context, Packet, Task<bool>>, and Packet must be an IPacket
    protected void Register<Packet>(Func<Context, Packet, Task<bool>> task) where Packet : IPacket
    {
        _funcTasks.Add(typeof(Packet), task);
    }


I don't know how to store that in a dictionary
private Dictionary<Type, Func<Context, IPacket, Task<bool>>> _funcTasks = new ();

This is not valid, despite Packet being an IPacket
AAkseli8/20/2022
_funcTasks.Add(typeof(Packet), Unsafe.As<Func<Context,IPacket,Task<bool>>>(task));
OOrannis8/20/2022
You can't
OOrannis8/20/2022
Not without doing horrible code like Akseli showed (seriously don't do that)
OOrannis8/20/2022
This about it this way:
class P1 : IPacket {}
class P2 : IPacket {}

var dictionary = new  Dictionary<Type, Func<Context, IPacket, Task<bool>>>();
var func = (P2 p2) => { ... /* Something that only P2 can do */ };
dictionary.Add(typeof(P2), func); // Assume this worked.
dictionary[typeof(P2)](null, new P1()); // Well, the dictionary has `Func<Context, IPacket, Task<bool>>`s, so this should be fine, right?
SStay8/20/2022
Oh yeah i get it. I guess that ill need to save the MethodInfo instead
SStay8/20/2022
Thanks
DDusty8/21/2022
U could also store a delegate
AAkseli8/21/2022
if its used correctly its completely safe and works, its the correct solution if you care about performance
AAkseli8/21/2022
another solution is to just
_funcTasks.Add(typeof(Packet), (context, pkt, t) => task(context, pkt as Packet, t));
Ccanton78/21/2022
Use a proper cast, not as, if you don't expect the cast to fail
OOrannis8/21/2022
No it is not. The proper solution is to design a type safe architecture. And as canton said, as is a bad idea in that nested lambda example
AAkseli8/21/2022
sometimes you need to sacrifice safety for performance, besides its completely safe to use in this case, it can never fail in the current context
OOrannis8/21/2022
That's a very bold assumption. Never assume that someone, particularly someone who is not super familiar with type safety, is going to use it completely correctly
AAkseli8/21/2022
thats why i said in the current context
OOrannis8/21/2022
Recommending unsafe code to anyone not in #allow-unsafe-blocks or #advanced, especially without any explanation of why it could work, or how it should never be misused, or how you need to actually use reflection to get it back to the original function type, is a very bad idea
AAkseli8/21/2022
agree with that, my bad.
OOrannis8/21/2022
Like, seriously, if someone isn't understanding these dangers, don't just post a code snippet on getting around the type checker
AAkseli8/21/2022
well this is a safe solution, it cant cause any runtime errors, only a null exception from the other side if as is returns null
AAkseli8/21/2022
also what do you mean you need to use reflection to get it back to original type? Using Unsafe.As thing you dont need to do that
OOrannis8/21/2022
If you ever need to extract that lambda again, then yes you need to use reflection to get a Type you can Unsafe.As on
AAkseli8/21/2022
if they extract it from the dictionary they can use it as <Context,IPacket,Task> it will work
OOrannis8/21/2022
Caches often need to return things beyond immediate usage
OOrannis8/21/2022
Without seeing the rest of Stay's code, we can't say for sure how the values in this dictionary are going to be used
AAkseli8/21/2022
we can assume what he wanted from what he said
OOrannis8/21/2022
All that was said was storing in a dictionary
OOrannis8/21/2022
Not how it would be retrieved
AAkseli8/21/2022
thats out of context of the question
AAkseli8/21/2022
he asked how to store it
OOrannis8/21/2022
It's important to the context of the question if you're making a recommendation that will significantly affect the storage of the thing
OOrannis8/21/2022
Like, you're assuming that there's only reference types implementing IPacket
AAkseli8/21/2022
that doesnt matter because when you retrieve the func with IPacket signature, the value type will be boxed to IPacket when calling
AAkseli8/21/2022
the method will still work as its supposed
OOrannis8/21/2022
... No
OOrannis8/21/2022
If the func takes a concrete value type, and you pass a boxed value type, it's not magically unboxed
OOrannis8/21/2022
It's just broken
AAkseli8/21/2022
yes...

if you had a value type

Packet pkt; that implements IPacket

and you call a method with IPacket parameter it will be boxed
OOrannis8/21/2022
You're not calling a method with an IPacket parameter
OOrannis8/21/2022
You Unsafe.As'd it to be such, but it doesn't actually have that parameter
AAkseli8/21/2022
but you are retrieving it as IPacket parameter
AAkseli8/21/2022
from the dictionary
OOrannis8/21/2022
That's the problem
OOrannis8/21/2022
You end up passing a boxed Packet to something that takes an unboxed packet, and nothing is going to do that unbox
OOrannis8/21/2022
It'll just fail
AAkseli8/21/2022
then you would just have to have packet as a class
AAkseli8/21/2022
sure its an assumption, could ask the OP for more details
OOrannis8/21/2022
All of these caveats are why you should be careful when making recommendations of unsafe code
OOrannis8/21/2022
Especially in a channel where the nuances and implications of it are probably not well understood
Ccanton78/21/2022
Especially given that the alternatives to using Unsafe here are pretty painless and cheap
AAkseli8/21/2022
using Reflection is not cheap..
Ccanton78/21/2022
You don't need to use reflection here
AAkseli8/21/2022
- MethodInfo is reflection
Ccanton78/21/2022
Who said to use MethodInfo?
AAkseli8/21/2022
well, thats what the OP took
Ccanton78/21/2022
Sure, but there are solutions which don't require reflection or Unsafe here
AAkseli8/21/2022
like this
Ccanton78/21/2022
Yep (although don't use as). Or ones which cast the whole delegate to Delegate/object and back. Those are the sorts of solutions to push IMO
OOrannis8/21/2022
Yes, though as does not work there
OOrannis8/21/2022
Literally won't compile
AAkseli8/21/2022
it works
OOrannis8/21/2022
No it doesn't
AAkseli8/21/2022
atleast it should, ive used this technique before
AAkseli8/21/2022
maybe you need to add a type
OOrannis8/21/2022
Packet is not known to be a reference or value type
OOrannis8/21/2022
Can't use as on it
Ccanton78/21/2022
as is the wrong thing to use there anyway. If you mess up and the cast fails, you get null passed to the delegate, which is probably going to cause an NRE somewhere down the line, and you'll have fine tracing it back to the bad cast. If you use a proper cast, you get a nice InvalidCastException at that point, telling you what you did wrong
AAkseli8/21/2022
right, yeah you need to use a cast, i didnt check if it compiles or not just threw it from memory on my phone
SStay8/21/2022
I'm back. Yeah, i understand why what i was trying to do yesterday at 3 am doesn't work now

Anyways, here is a bit of context of what i'm trying to do in case that there is a solution better of what i currently have

I'm trying to add a easier and faster way of handling packets in my application, i thought of doing something like this

public MyPacketHandler : BasePacketHandler<Context>
{
    public MyPacketHandler()
    {
        Register<IHeartbeatC2SP>(HeartbeatC2SP_Handle);
    }

    private async Task<bool> HeartbeatC2SP_Handle(Context context, IHeartbeatC2SP)
    {
        // ...
    }
}


I know that i could do this
    protected void Register<Packet>(Func<Context, IPacket, Task<bool>> task) where Packet : IPacket
    {
        _funcTasks.Add(typeof(Packet), task);
    }

But that would require of casting on the handle method
var handshake = (IHandshakeC2SP) packet;

Which is not the end of the world, but it's what i'm trying to prevent
Ccanton78/21/2022
Yeah looks familiar. I'd probably wrap the delegate in another one which does the cast for you, like this https://discord.com/channels/143867839282020352/1010669940887539722/1010860845410029599 but with a proper cast rather than as
Ccanton78/21/2022
If it's networking, you're not going to care about the extra delegate invocation
SStay8/21/2022
Yeah, that's probably also faster than MethodInfo, right?
Ccanton78/21/2022
Probably not too far off - the cost there is getting the MethodInfo, but you're caching it. But yeah slightly cheaper, and certainly more readable and safer
SStay8/21/2022
But MethodInfo requires an object[], which would box the struct behind IPacket
SStay8/21/2022
(as far as i know)
Ccanton78/21/2022
Yep, but that's being boxed anyway. Interfaces are reference types
Ccanton78/21/2022
So assigning the struct to IPacket boxes it
SStay8/21/2022
Oh i didn't know that
SStay8/21/2022
Thanks, i'll use this solution then
SStay8/21/2022
How do i mark this thing as solved? First time using discord threads
Ccanton78/21/2022
$close
Ccanton78/21/2022
Not that, heh
AAccord8/21/2022
Ask the thread owner or member with permission to close this!
Ccanton78/21/2022
Ah, with /close
SStay8/21/2022
nice
Image
AAccord8/21/2022
✅ This post has been marked as answered!
SSergio8/21/2022
This is absolutely not correct. It is not, by any means "completely safe". It may work today in some cases, but it's undefined behavior and it might (and will) break down in several scenarios. Yes, even if you're using "the right types". It's not something that you should be using even if you think you know how it works, it's just not correct code. Nobody should use that, period
AAkseli8/21/2022
we already had this conversation and went over the cases where it wont work
SSergio8/21/2022
Right. So you're aware that code is not valid. Then don't recommend it to people online asking for help
SSergio8/21/2022
Actually wait, no, I think we're not talking about the same thing here
AAkseli8/21/2022
its not valid in all cases, no
SSergio8/21/2022
No. In no case
SSergio8/21/2022
There is no case where that code is safe
SSergio8/21/2022
I know you think you know. You don't
AAkseli8/21/2022
add a class constraint to Packet and its safe
AAkseli8/21/2022
if you dont do changes
SSergio8/21/2022
it is not
SSergio8/21/2022
I literally had this conversation with the actual principal architect of the .NET runtime (Jan). This is not safe, ever. Period