C#•2y ago

Store generic Func T inside a colletion [Answered]

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

72 Replies

Stay•2y ago

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);
    }

    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 ();

private Dictionary<Type, Func<Context, IPacket, Task<bool>>> _funcTasks = new ();

This is not valid, despite Packet being an IPacket

Akseli•2y ago

_funcTasks.Add(typeof(Packet), Unsafe.As<Func<Context,IPacket,Task<bool>>>(task));

_funcTasks.Add(typeof(Packet), Unsafe.As<Func<Context,IPacket,Task<bool>>>(task));

333fred•2y ago

You can't Not without doing horrible code like Akseli showed (seriously don't do that) 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?

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?

Stay•2y ago

Oh yeah i get it. I guess that ill need to save the MethodInfo instead Thanks

Dusty•2y ago

U could also store a delegate

Akseli•2y ago

if its used correctly its completely safe and works, its the correct solution if you care about performance another solution is to just

_funcTasks.Add(typeof(Packet), (context, pkt, t) => task(context, pkt as Packet, t));

_funcTasks.Add(typeof(Packet), (context, pkt, t) => task(context, pkt as Packet, t));

canton7•2y ago

Use a proper cast, not as, if you don't expect the cast to fail

333fred•2y ago

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

Akseli•2y ago

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

333fred•2y ago

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

Akseli•2y ago

thats why i said in the current context

333fred•2y ago

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

Akseli•2y ago

agree with that, my bad.

333fred•2y ago

Like, seriously, if someone isn't understanding these dangers, don't just post a code snippet on getting around the type checker

Akseli•2y ago

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 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

333fred•2y ago

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

Akseli•2y ago

if they extract it from the dictionary they can use it as <Context,IPacket,Task> it will work

333fred•2y ago

Caches often need to return things beyond immediate usage 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

Akseli•2y ago

we can assume what he wanted from what he said

333fred•2y ago

All that was said was storing in a dictionary Not how it would be retrieved

Akseli•2y ago

thats out of context of the question he asked how to store it

333fred•2y ago

It's important to the context of the question if you're making a recommendation that will significantly affect the storage of the thing Like, you're assuming that there's only reference types implementing IPacket

Akseli•2y ago

that doesnt matter because when you retrieve the func with IPacket signature, the value type will be boxed to IPacket when calling the method will still work as its supposed

333fred•2y ago

... No If the func takes a concrete value type, and you pass a boxed value type, it's not magically unboxed It's just broken

Akseli•2y ago

yes... if you had a value type Packet pkt; that implements IPacket and you call a method with IPacket parameter it will be boxed

333fred•2y ago

You're not calling a method with an IPacket parameter You Unsafe.As'd it to be such, but it doesn't actually have that parameter

Akseli•2y ago

but you are retrieving it as IPacket parameter from the dictionary

333fred•2y ago

That's the problem You end up passing a boxed Packet to something that takes an unboxed packet, and nothing is going to do that unbox It'll just fail

Akseli•2y ago

then you would just have to have packet as a class

MODiX•2y ago

Orannis#3333

Like, you're assuming that there's only reference types implementing IPacket

Quoted by

<@!269674004065943552> from #Store generic Func T inside a colletion (click here)

React with ❌ to remove this embed.

Akseli•2y ago

sure its an assumption, could ask the OP for more details

333fred•2y ago

All of these caveats are why you should be careful when making recommendations of unsafe code Especially in a channel where the nuances and implications of it are probably not well understood

canton7•2y ago

Especially given that the alternatives to using Unsafe here are pretty painless and cheap

Akseli•2y ago

using Reflection is not cheap..

canton7•2y ago

You don't need to use reflection here

Akseli•2y ago

- MethodInfo is reflection

canton7•2y ago

Who said to use MethodInfo?

Akseli•2y ago

well, thats what the OP took

canton7•2y ago

Sure, but there are solutions which don't require reflection or Unsafe here

Akseli•2y ago

like this

canton7•2y ago

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

333fred•2y ago

Yes, though as does not work there Literally won't compile

Akseli•2y ago

it works

333fred•2y ago

No it doesn't

Akseli•2y ago

atleast it should, ive used this technique before maybe you need to add a type

333fred•2y ago

Packet is not known to be a reference or value type Can't use as on it

canton7•2y ago

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

Akseli•2y ago

right, yeah you need to use a cast, i didnt check if it compiles or not just threw it from memory on my phone

Stay•2y ago

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)
    {
        // ...
    }
}

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);
    }

    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;

var handshake = (IHandshakeC2SP) packet;

Which is not the end of the world, but it's what i'm trying to prevent

canton7•2y ago

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

MODiX•2y ago

Akseli#8820

another solution is to just

_funcTasks.Add(typeof(Packet), (context, pkt, t) => task(context, pkt as Packet, t));

_funcTasks.Add(typeof(Packet), (context, pkt, t) => task(context, pkt as Packet, t));

Quoted by

<@!660066004059029524> from #Store generic Func T inside a colletion (click here)

React with ❌ to remove this embed.

canton7•2y ago

If it's networking, you're not going to care about the extra delegate invocation

Stay•2y ago

Yeah, that's probably also faster than MethodInfo, right?

canton7•2y ago

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

Stay•2y ago

But MethodInfo requires an object[], which would box the struct behind IPacket (as far as i know)

canton7•2y ago

Yep, but that's being boxed anyway. Interfaces are reference types So assigning the struct to IPacket boxes it

Stay•2y ago

Oh i didn't know that Thanks, i'll use this solution then How do i mark this thing as solved? First time using discord threads

canton7•2y ago

$close Not that, heh

Accord•2y ago

Ask the thread owner or member with permission to close this!

canton7•2y ago

Ah, with /close

Stay•2y ago

nice

Accord•2y ago

✅ This post has been marked as answered!

Sergio•2y ago

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

Akseli•2y ago

we already had this conversation and went over the cases where it wont work

Sergio•2y ago

Right. So you're aware that code is not valid. Then don't recommend it to people online asking for help Actually wait, no, I think we're not talking about the same thing here

Akseli•2y ago

its not valid in all cases, no

Sergio•2y ago

No. In no case There is no case where that code is safe I know you think you know. You don't

Akseli•2y ago

add a class constraint to Packet and its safe if you dont do changes

Sergio•2y ago

it is not I literally had this conversation with the actual principal architect of the .NET runtime (Jan). This is not safe, ever. Period

Akseli•2y ago

okay, i believe you, can you tell me why its not safe? so i can learn

Sergio•2y ago

"I think the highest risk of incompatibilities come from codegen optimizations. Some of the advanced codegen optimizations assume type safety. Your code may break in odd ways with type safety violations."

"Yes. For example: The delegate can be bound virtually. In future, we may do delegate inlining and speculative devirtualization for this case. Is it going to work properly when some of the signatures involved violate type safety? I would not bet on it."

TLDR: the runtime and compiler assume type safety is never broken. Even if you're using "types correctly" here, things can still break down at any time because this is fundamentally undefined behavior, and other components might be doing things with some assumptions which you're breaking there Just don't do this 🙂

Akseli•2y ago

hmm makes sense i could argue against it but id rather take the L to learn L for Learn

Gaming

Programming

Store generic Func T inside a colletion [Answered]