Faithlife / FaithlifeData

Helpers for querying ADO.NET-compatible databases.
https://faithlife.github.io/FaithlifeData/
MIT License
6 stars 4 forks source link

Add DbParameters methods for collections and DTOs #15

Closed TyMick closed 3 years ago

TyMick commented 3 years ago

Hello! 😊 This PR adds several new methods to DbParameters (and modifies two existing ones):

Collections (resolves #7):

public static DbParameters Create(string name, IEnumerable<object?> values);
public static DbParameters Create(Func<int, string> name, IEnumerable<object?> values);

public DbParameters Add(string name, IEnumerable<object?> values);
public DbParameters Add(Func<int, string> name, IEnumerable<object?> values);

DTOs (resolves #8):

I added an optional string? suffix parameter to each DTO method to make the "collections of DTOs" methods (#9) more succinct.

// Modified
public static DbParameters FromDto(object dto, string? suffix = null);
public DbParameters AddDto(object dto, string? suffix = null);

// New
public static DbParameters FromDto(string name, object dto, string? suffix = null);
public static DbParameters FromDto(Func<string, string> name, object dto, string? suffix = null);

public DbParameters AddDto(string name, object dto, string? suffix = null);
public DbParameters AddDto(Func<string, string> name, object dto, string? suffix = null);

Collections of DTOs (resolves #9):

public static DbParameters FromDtos(IEnumerable<object> dtos);
public static DbParameters FromDtos(string name, IEnumerable<object> dtos);
public static DbParameters FromDtos(Func<string, int, string> name, IEnumerable<object> dtos);

public DbParameters AddDtos(IEnumerable<object> dtos);
public DbParameters AddDtos(string name, IEnumerable<object> dtos);
public DbParameters AddDtos(Func<string, int, string> name, IEnumerable<object> dtos);

I also added tests for each of these methods along the way.

Is adding the optional string? suffix parameter to the original DTO methods a good solution? Would it be better to restore the original versions and add additional suffix overloads? Or would it be best to throw out the suffix params altogether and just do the extra work in the "collections of DTOs" methods?

ejball commented 3 years ago

Thank you! I will try to review these changes soon!

TyMick commented 3 years ago

There, that should be much better! Thanks especially for catching my creating all those extra immutable objects. Since the underlying IReadOnlyList is storing tuples, a value type, the operation of copying such a read-only list has O(n) complexity, right? since it's having to copy every individual value? Which means, by extension, that when I created a new DbParameters in every single loop inside my FromMany method, that gave it O(n²) complexity?

By contrast, the new version of FromMany that adds each new value to a mutable List and waits till the end to create the returned DbParameters should have O(n) complexity, right?

ejball commented 3 years ago

Right!