Consider the case that ObjectReader is explicitly given member names that don't exist on the underlying type.
This line in ObjectReader: this.effectiveTypes[i] = memberType ?? typeof(object); does not throw an exception when memberType is null (not found on the concrete/delegate type), when that could be a way to let the developer know they goofed and gave a member that doesn't exist on the type. But it doesn't. This seems to hint that it intends on doing something nicer with missing members.
This line in GetValues: for (int i = 0; i < count; i++) values[i] = accessor[current, members[i]] ?? DBNull.Value; wants to swap out nulls for DBNull, but since we don't have the member on the type, we are definitely going to crash with ArgumentOutOfRangeException; the indexer explicitly throws it when we mismatch on name. This seems to contradict point 1; nothing nicer is happening for missing members, we just find out further along that we got it wrong by crashing.
So why not throw in the constructor?
This isn't that straightforward because we have to support dynamic types, too, which already don't have member information and rely on explicit members -- all while the reader currently expects every object to have the same set of members, because a single Type is expected to be passed along (we can't scan the enumerable looking for missing members on the dynamic type, for obvious reasons).
I'm guessting that we aren't meant to have two different sets of behaviour (throw in the ctor if we provide mismatched types on a concrete/delegate accessor, and throw on GetValues (during table load) if we provide mismatched types on a dynamic accessor).
It seems like there are two possible fixes:
Opportunistically throw in the ctor in all cases where the user passes members not found on the specified type, with something informative.
Make GetValues resilient to missing members (i.e. via #66) and return a null corresponding to the effectiveType of object in cases of missing members. Essentially a no-op.
Tangentially, #67 could be fixed, if option 2 is the right choice, with basically: throw in the ctor if we end up effectively with no members at all. This would be the general case of accidentally passing in List<object>, applying in more circumstances of failure.
I can implement either fix, or no fix, but don't know which direction to take.
Consider the case that
ObjectReaderis explicitly given member names that don't exist on the underlying type.This line in
ObjectReader:this.effectiveTypes[i] = memberType ?? typeof(object);does not throw an exception whenmemberTypeis null (not found on the concrete/delegate type), when that could be a way to let the developer know they goofed and gave a member that doesn't exist on the type. But it doesn't. This seems to hint that it intends on doing something nicer with missing members.This line in
GetValues:for (int i = 0; i < count; i++) values[i] = accessor[current, members[i]] ?? DBNull.Value;wants to swap out nulls forDBNull, but since we don't have the member on the type, we are definitely going to crash withArgumentOutOfRangeException; the indexer explicitly throws it when we mismatch on name. This seems to contradict point 1; nothing nicer is happening for missing members, we just find out further along that we got it wrong by crashing.So why not throw in the constructor?
This isn't that straightforward because we have to support dynamic types, too, which already don't have member information and rely on explicit members -- all while the reader currently expects every object to have the same set of members, because a single Type is expected to be passed along (we can't scan the enumerable looking for missing members on the dynamic type, for obvious reasons).
I'm guessting that we aren't meant to have two different sets of behaviour (throw in the ctor if we provide mismatched types on a concrete/delegate accessor, and throw on
GetValues(during table load) if we provide mismatched types on a dynamic accessor).It seems like there are two possible fixes:
Opportunistically throw in the ctor in all cases where the user passes members not found on the specified type, with something informative.
Make
GetValuesresilient to missing members (i.e. via #66) and return anullcorresponding to theeffectiveTypeofobjectin cases of missing members. Essentially a no-op.Tangentially, #67 could be fixed, if option 2 is the right choice, with basically: throw in the ctor if we end up effectively with no members at all. This would be the general case of accidentally passing in
List<object>, applying in more circumstances of failure.I can implement either fix, or no fix, but don't know which direction to take.