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

Consider ditching Activator.CreateInstance #5

Closed
dazinator opened this issue Oct 31, 2017 · 2 comments
Closed

Consider ditching Activator.CreateInstance #5

dazinator opened this issue Oct 31, 2017 · 2 comments

Comments

@dazinator
Copy link
Contributor

You could ditch Activator.CreateInstance for a public default constructor constraint on type T. i.e:


            private IList<T> MapToList<T>(DbDataReader dr) where T: new()
            {
                var objList = new List<T>();
                var props = typeof(T).GetRuntimeProperties();

                var colMapping = dr.GetColumnSchema()
                    .Where(x => props.Any(y => y.Name.ToLower() == x.ColumnName.ToLower()))
                    .ToDictionary(key => key.ColumnName.ToLower());

                if (dr.HasRows)
                {
                    while (dr.Read())
                    {
                        T obj = new T();
                        foreach (var prop in props)
                        {
                            var val = dr.GetValue(colMapping[prop.Name.ToLower()].ColumnOrdinal.Value);
                            prop.SetValue(obj, val == DBNull.Value ? null : val);
                        }
                        objList.Add(obj);
                    }
                }
                return objList;
            }

You could also cache the props for type T, to save calling GetRuntimeProperties on every call you could grab them from the cache if they are there. Not sure how much that would save as I haven't checked to see if GetRuntimeProperties does caching of it's own.

@sp00ky
Copy link

sp00ky commented Dec 7, 2017

Seems like a candidate for a pull request.

@snickler
Copy link
Owner

I was thinking of something in regards to this. I believe we should switch this to use an Expression Tree instead of reflection. We'll get some great speed improvements out of it. I can look into it over the next few days and see what I can whip up

skutnarSDS added a commit to skutnarSDS/EFCore-FluentStoredProcedure that referenced this issue Feb 6, 2020
…r#25)

- Added method ExecuteStoredProcAsync that takes params Action<SprocResults>[]
- In MapToList: replaced usage of Activator.CreateInstance in MapToList with new T() (issue snickler#5)
- In MapToList: tweaked string compares to follow better practices
- Formatting
- Fixes to XML documentation
- Finor refactoring / readability fixes
- Removed unused parameters from some methods:
  - WithSqlParam that takes SqlParameter - removed unused paramName
  - ExecuteStoredNonQuery                - removed unused commandBehavior
  - ExecuteStoredNonQueryAsync           - removed unused commandBehavior
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

3 participants