Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ValueTask<T> not working in Auto Interface Implementation, please provide support (System.Threading.Tasks.Extensions nuget package) #435

Closed
keerthirajap opened this issue Feb 19, 2020 · 3 comments

Comments

@keerthirajap
Copy link

Describe the feature

Please provide support for ValueTask used along with Auto Interface Implementation. ValueTask installed through System.Threading.Tasks.Extensions nuget package

Is this feature related to a problem, describe

User Model

    public class User
    {
        public long UserId { get; set; }

        public string UserName { get; set; }

        public string FirstName { get; set; }

        public string LastName { get; set; }
    }

IUserRepository.cs


 public interface IUserRepository
    {
        [Sql(@"SELECT [UserId]
                  ,[UserName]
                  ,[FirstName]
                  ,[LastName]
              FROM[dbo].[User]")]
        Task<List<User>> TaskGetAllUsersAsync();             **// This works**

        [Sql(@"SELECT [UserId]
                  ,[UserName]
                  ,[FirstName]
                  ,[LastName]
              FROM[dbo].[User]")]
        ValueTask<List<User>> ValueTaskGetAllUsersAsync();   **// This not working, return null value**
    }

Program.cs


        private static async Task Main(string[] args)
        {
            DbConnection c = new SqlConnection("");
            IUserRepository i = c.As<IUserRepository>();

            List<User> users = new List<User>();
            
            **// This works**
            users = await i.TaskGetAllUsersAsync();

           **// This not working, return null value**
            users = await i.ValueTaskGetAllUsersAsync();

            Console.ReadKey();
        }

Additional context

@andymac4182
Copy link
Contributor

I am curious. What is the benefit of ValueTask over Task? My understanding was ValueTask was for situations where the function has the chance it would be executed synchronously. Is there another way it can be useful for this situation?

@lawrencek76
Copy link
Collaborator

@andymac4182 I think you are correct in your understanding. There may be other opportunities to expose valuetask for example BeginDbTransactionAsync Now returns valuetask so exposing those from the underlying DbCommand does make sense wherever possible.

See dotnet/runtime#28596

As far as adding It to the auto interface implementation I don’t see why it can’t be done but its seems like only a very few dB providers maybe none at this point would benefit.

@jonwagner
Copy link
Owner

Interface methods all go through paths that are almost guaranteed to do I/O.

Open => Exec => Read => Close

The Insight methods all return Task, so there's no benefit from supporting ValueTask.

I toyed around with supporting ValueTask as a return type. It's possible to do it for simplicity, but it would result in an additional allocation on each return.

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

No branches or pull requests

4 participants