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

Non-generic IDatumConverter & IDatumConverterFactory #135

Merged
merged 5 commits into from
Sep 20, 2013
Merged

Conversation

mfenniak
Copy link
Owner

Add non-generic interfaces for IDatumConverter and IDatumConverterFactory. This is likely a pre-req for removing reflection in some other issues (#69, #67, #66) and fixing the dynamic invocation in issue #133.

public interface IDatumConverter
{
    object ConvertDatum(Datum datum);
    Datum ConvertObject(object @object);
}

public interface IDatumConverter<T> : IDatumConverter
{
    new T ConvertDatum(Datum datum);
    Datum ConvertObject(T @object);
}

public interface IDatumConverterFactory
{
    bool TryGet(Type datumType, IDatumConverterFactory rootDatumConverterFactory, out IDatumConverter datumConverter);
    bool TryGet<T>(IDatumConverterFactory rootDatumConverterFactory, out IDatumConverter<T> datumConverter);
}

@ghost ghost assigned mfenniak Sep 20, 2013
Previous revision allowed AbstractDatumConverter to automatically handle
nulls in the non-generic ConvertDatum and ConvertObject methods; this
would have allowed null values to be used for non-nullable datum
converters in some cases.  To address this, the abstract class was split
into two types; one which works for reference types (nullable), one which
works for value types (non-nullable), and NullableDatumConverter was
updated to not use the abstract base class so that it can intentionally do
null value types correctly.
mfenniak added a commit that referenced this pull request Sep 20, 2013
Non-generic IDatumConverter & IDatumConverterFactory
@mfenniak mfenniak merged commit d1b86e7 into master Sep 20, 2013
@mfenniak mfenniak deleted the issue_135 branch September 20, 2013 17:36
mfenniak added a commit that referenced this pull request Sep 20, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant