Skip to content

Conversation

fsarab57
Copy link

Fix AddDispatchR when dispatchR and handlers are in same project and remove unnecessary cast from publish method

…remove unnecessary cast from publish method
@@ -36,9 +36,7 @@ public IAsyncEnumerable<TResponse> CreateStream<TRequest, TResponse>(IStreamRequ

public async ValueTask Publish<TNotification>(TNotification request, CancellationToken cancellationToken) where TNotification : INotification
{
var notificationsInDi = serviceProvider.GetRequiredService<IEnumerable<INotificationHandler<TNotification>>>();

var notifications = Unsafe.As<INotificationHandler<TNotification>[]>(notificationsInDi);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bro, this piece of code is necessary because when you use foreach on an IEnumerable, it causes boxing, which leads to memory allocation on the heap. With this approach, we avoid using foreach directly on IEnumerable.

More information: https://youtu.be/7w4ho3WzKss?list=PLGiSgN3ODieILgFQN1puu-ey9guWwnxGX&t=3280

.Where(p =>
var allTypes = configurationOptions.Assemblies.SelectMany(x => x.GetTypes())
.Distinct()
.Where(type => type is { IsClass: true, IsAbstract: false })
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great, but I feel like we’ve drifted away from the original goal. We wanted to avoid picking up interfaces, but now we’re explicitly saying it has to be a class and not abstract.

I think if we just exclude interfaces and allow everything else, we could also support other types like structs.

What do you think?

@hasanxdev
Copy link
Owner

Do we have any updates on this? @fsarab57

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants