In our application we have complex objects with many nesting levels, each with various properties that are important to the business.  We currently serialize and persist these objects along with their corresponding workflows (Windows Workflow 4.5), but, due to issues with updating active workflows (WF4.5 Dynamic Update - Issues when updating large, long-running flows with custom activities), we are making a plan for storing the important properties in our SqlServer database in preparation of moving away from Windows Workflow.Each object is identified by a Filetype and Filenumber, which the business uses to move the data through a long-running Business Process. Some of the child objects and their properties are specific to each Filetype/Filenumber (for example, Customer Name), while others can occur multiple times for each Filetype/Filenumber (for example, objects representing Ad-hoc correspondence).I've created two custom attributes, [Savable] and [SavableWithContext], and placed them on child objects and their properties where appropriate.  I'm then using reflection and recursion to walk any complex object sent in, and saving those properties and their values where marked. If they are marked [Savable], I simply save the property name and value, but if they're marked [SavableWithContext], I tack on each parent-object's name as I walk down the object to basic types, so that in the database we can identify which properties those values correspond with. In the logic layer, I build a list with these important properties, which I then send down to our DAL to insert/update as appropriate.In tests, this code works, and takes about 3 seconds with an object that builds a list of 180 CaseDataItems.  This is much faster than the current workflow serialization, but in production there will be many more properties so I'm looking at ways to speed this up.  I'm also looking into having this saving happen asynchronously, but because I'm using a lot of reflection/recursion, I'm sure there are many improvements to be made here as well. I've also decided to just store each property, regardless of whether it has a value or not, so there are a lot of empty strings in the Database.  My thinking is that its faster to just store rather than query the database first, which I would have to do in case there was originally a value, but then the user cleared the textbox at a later point in the process.  Would love a second opinion on whether this is the way to go or not.(Also, I should note that I'm a junior dev and this has been my most complex task yet, so there may be many basic mistakes that I've made)The Gist: I'm using reflection and recursion to store complex data objects, and would like to know:Are there any noobish mistakes/improvements I can make with this code?Are there some ways to speed this up?Should I worry about handling empty values rather than storing them in the db no matter what? if so, then:Is there a quick way to handle properties that previously had a value in the database, but now are null/empty from the user?I've left some of my comments for now, I wasn't sure if I should remove them or not.  If you need to see other parts of the code, or an example of what an object looks like, let me know!Main Code:Logic Layer:public static class ObjectPropertyFunctions{    private static List<CaseDataItem> caseDataItems;    private static string parentContextName;    public static void SaveProperties<T>(T instance)    {        try        {            caseDataItems = new List<CaseDataItem>();            AddPropertiesRecursive(typeof(T), instance);            CaseDataServices.SaveCaseData(caseDataItems);        }        catch (Exception ex)        {            // Just log the message, but for now don't alert the User.            Logging.logger.Error(ex.Message, ex);        }    }    private static void AddPropertiesRecursive<T>(Type classType, T instance)    {        bool needsContext = false;        IEnumerable<PropertyInfo> propsToAdd = instance.GetType().GetProperties().Where(            prop => (Attribute.IsDefined(prop, typeof(SavableAttribute)) ||                    (Attribute.IsDefined(prop, typeof(SavableWithContextAttribute)))));        foreach (PropertyInfo property in propsToAdd)        {            needsContext = Attribute.IsDefined(property, typeof(SavableWithContextAttribute));            // Set parentContextname on first object iteration, and skip once base type is hit            if (needsContext && !(BasicTypes.List.Any(x => x.IsAssignableFrom(property.PropertyType))))            {                if (parentContextName.IsNullOrEmpty())                {                    parentContextName += property.Name;                }                else                {                    parentContextName += . + property.Name;                }            }            // Add basic types, while recursing properties that are objects            if (BasicTypes.List.Any(x => x.IsAssignableFrom(property.PropertyType)))            {                if (property.PropertyType.IsEnum)                {                    AddEnum(property, instance, needsContext ? true : false);                }                else                {                    AddBasicType(property, instance, needsContext ? true : false);                }            }            else if (property.PropertyType.IsClass)            {                object value = property.GetValue(instance);                if (value != null)                {                    string tempParent = parentContextName.IsNullOrEmpty() ?  : parentContextName + .;                    AddPropertiesRecursive(property.PropertyType, value);                }            }        }        // Only remove the parentcontextname if one was added        if (needsContext)        {            RemoveObjectFromParentContextName();        }    }    private static void RemoveObjectFromParentContextName()    {        if (parentContextName.IsNullOrEmpty())            return;        // Determine whether to reset the string (there was only one object in it), or subtract the last one (there are still objects above)        int periods = parentContextName.Count(x => x == '.');        if (periods > 0)        {            string[] parentContextParts = parentContextName.Split(new char[] { '.' }, StringSplitOptions.RemoveEmptyEntries);            parentContextParts = parentContextParts.Take(parentContextParts.Count() - 1).ToArray();            parentContextName = string.Join(., parentContextParts);        }        else        {            parentContextName = ;        }    }    private static void AddBasicType(PropertyInfo property, object instance, bool saveWithContext = false)    {        TypeCode typeCode = Type.GetTypeCode(property.PropertyType);        switch (typeCode)        {            case TypeCode.String:                caseDataItems.Add(new CaseDataItem()                {                    DataName = (saveWithContext ? parentContextName + . : ) + property.Name,                    DataType = STRING,                    StringValue = (string)property.GetValue(instance, null)                });                break;            case TypeCode.DateTime:                caseDataItems.Add(new CaseDataItem()                {                    DataName = (saveWithContext ? parentContextName + . : ) + property.Name,                    DataType = DATETIME,                    DateTimeValue = (DateTime)property.GetValue(instance, null)                });                break;            case TypeCode.Decimal:                caseDataItems.Add(new CaseDataItem()                {                    DataName = (saveWithContext ? parentContextName + . : ) + property.Name,                    DataType = DECIMAL,                    DecimalValue = (decimal)property.GetValue(instance, null),                });                break;            case TypeCode.Boolean:                caseDataItems.Add(new CaseDataItem()                {                    DataName = (saveWithContext ? parentContextName + . : ) + property.Name,                    DataType = BOOLEAN,                    BooleanValue = (bool)property.GetValue(instance, null)                });                break;            case TypeCode.Int16:            case TypeCode.Int32:            case TypeCode.Int64:                caseDataItems.Add(new CaseDataItem()                {                    DataName = (saveWithContext ? parentContextName + . : ) + property.Name,                    DataType = INTEGER,                    IntegerValue = (int)property.GetValue(instance, null)                });                break;            default: break;        }    }    private static void AddEnum(PropertyInfo property, object instance, bool saveWithContext = false)    {        object propertyValue = property.GetValue(instance, null);        string enumString = property.ToString();        caseDataItems.Add(new CaseDataItem()        {            DataName = (saveWithContext ? parentContextName + . : ) + property.Name,            DataType = STRING,            StringValue = propertyValue.ToString()        });    }}Data Access Layer:    public static bool SaveCaseData(List<CaseDataItem> caseDataItems)    {        bool ok = false;        string fileType = Config.Settings.FileType;        string fileNumber = Config.Settings.FileNumber;        int totalcount = caseDataItems == null ? -1 : caseDataItems.Where(c => !c.DataName.IsNullOrEmpty() && !c.DataType.IsNullOrEmpty()).Count();        if (totalcount < 1)        {            Logging.logger.Warn(CaseDataItems input is null or empty. Nothing to save.);        }        else if (fileType.IsNullOrEmpty() || fileNumber.IsNullOrEmpty())        {            Logging.logger.Warn(Configuration does not have file type and number. Unable to save data.);        }        else        {            using (SqlConnection connection = new SqlConnection(dbConnectionString))            {                using (SqlCommand cmd = new SqlCommand())                {                    connection.Open();                    cmd.Connection = connection;                    int successCount = 0;                    cmd.CommandText =                        @UPDATE [Case].CaseData SET StringValue = @StringValue,                            IntegerValue = @IntegerValue,                            BooleanValue = @BooleanValue,                            DateTimeValue = @DateTimeValue,                            DecimalValue = @DecimalValue,                            UpdatedBy = @UserId,                            UpdatedDate = GetDate()                        WHERE DataName = @DataName               AND DataType = @DataType               AND CaseFTFNId = (SELECT TOP 1 CaseFTFNId                                     FROM [Case].CaseFTFN                                    WHERE FileType = @FileType                               AND FileNumber = @FileNumber)                     IF @@ROWCOUNT=0                        INSERT INTO [Case].CaseData (                            CaseFTFNId                            ,DataName                            ,DataType                            ,StringValue                            ,IntegerValue                            ,BooleanValue                            ,DateTimeValue                            ,DecimalValue                            ,CreatedBy                            ,CreatedDate                            ,UpdatedBy                            ,UpdatedDate)               VALUES ((SELECT TOP 1 CaseFTFNId                             FROM [Case].CaseFTFN                            WHERE FileType = @FileType                           AND FileNumber = @FileNumber)                             ,@DataName                            ,@DataType                            ,@StringValue                            ,@IntegerValue                            ,@BooleanValue                            ,@DateTimeValue                            ,@DecimalValue                            ,@UserId                            ,GetDate()                            ,@UserId                            ,GetDate());                    cmd.Parameters.AddWithValue(UserId, Environment.UserName);                    cmd.Parameters.AddWithValue(FileType, fileType);                    cmd.Parameters.AddWithValue(FileNumber, fileNumber);                    cmd.Parameters.Add(DataName, SqlDbType.VarChar);                    cmd.Parameters.Add(DataType, SqlDbType.VarChar);                    cmd.Parameters.Add(StringValue, SqlDbType.VarChar);                    cmd.Parameters.Add(IntegerValue, SqlDbType.Int);                    cmd.Parameters.Add(BooleanValue, SqlDbType.Bit);                    cmd.Parameters.Add(DateTimeValue, SqlDbType.DateTime);                    cmd.Parameters.Add(DecimalValue, SqlDbType.Decimal);                    foreach (CaseDataItem cdi in caseDataItems.Where(c => !c.DataName.IsNullOrEmpty() && !c.DataType.IsNullOrEmpty()))                    {                        cmd.Parameters[DataName].Value = cdi.DataName;                        cmd.Parameters[DataType].Value = cdi.DataType;                        cmd.Parameters[StringValue].Value = (object)cdi.StringValue ?? DBNull.Value;                        cmd.Parameters[IntegerValue].Value = (object)cdi.IntegerValue ?? DBNull.Value;                        cmd.Parameters[BooleanValue].Value = (object)cdi.BooleanValue ?? DBNull.Value;                        cmd.Parameters[DateTimeValue].Value = (object)cdi.DateTimeValue ?? DBNull.Value;                        cmd.Parameters[DecimalValue].Value = (object)cdi.DecimalValue ?? DBNull.Value;                        successCount += cmd.ExecuteNonQuery();                    }                }            }        }        return ok;    }Other objects I'm using in this code:BasicTypesstatic class BasicTypes{    public static readonly Type[] List;    static BasicTypes()    {        var types = new[]        {            typeof (Enum),            typeof (string),            typeof (bool),            typeof (int),            typeof (double),            typeof (DateTime),            typeof (TimeSpan),        };        var nullTypes = from t in types                        where t.IsValueType                        select typeof(Nullable<>).MakeGenericType(t);        List = types.Concat(nullTypes).ToArray();    }}Case Data Item public class CaseDataItem{    public CaseDataItem() { }    public bool HasData()    {        bool result = false;        try        {            result = !(DataId == 0                || DataName.IsNullOrEmpty() || DataType.IsNullOrEmpty()                 || UserId.IsNullOrEmpty());        }        catch (Exception ex)        {            Logging.logger.Error(ex.Message, ex);        }        return result;    }    public object GetValue()    {        switch (DataType)        {            case STRING:                return StringValue;            case INTEGER:                return IntegerValue;            case DATETIME:                return DateTimeValue;            case BOOLEAN:                return BooleanValue;            case DECIMAL:                return DecimalValue;            default:                return null;        }    }    public int DataId { get; set; }    public string DataName { get; set; }    public string DataType { get; set; }    public string UserId { get; set; }    public string StringValue { get; set; }    public int IntegerValue { get; set; }    public DateTime? DateTimeValue { get; set; }    public bool BooleanValue { get; set; }    public decimal DecimalValue { get; set; }    public DateTime? ModifiedDate { get; set; }       }
Saving complex business objects with reflection and recursion
c#;sql;recursion;sql server;reflection
RedundancyRedundant variablesYou have a few unused variables :private static void AddPropertiesRecursive<T>(Type classType, T instance)int successCount = 0;And lastly - bool ok = false;, SaveCaseData return bool but in reality it always returns false. It makes more sense to make the method void. As most save methods are of type void ,it's still fine to have boolean Save method but if the return value changes depending if the saving succeeded but that's not the case in your method.If you still want to keep the bool method for some reason you should just return false at the end.General redundancyif (property.PropertyType.IsEnum){    AddEnum(property, instance, needsContext ? true : false);}else{    AddBasicType(property, instance, needsContext ? true : false);}You don't need to manually check the value of needsContext and depending on that assign a boolean value as needsContext is a boolean variable anyway you can use that :if (property.PropertyType.IsEnum){    AddEnum(property, instance, needsContext);}else{    AddBasicType(property, instance, needsContext);}switch case with default case which contains only break is redundantdefault:break;Non nullable value type variables compared again null ?This doesn't makes much sense as value type variables cant have value of null anyway (unless marked as nullable - ?)cmd.Parameters[DecimalValue].Value = (object) cdi.DecimalValue ?? DBNull.Value;cmd.Parameters[IntegerValue].Value = (object)cdi.IntegerValue ?? DBNull.Value;You can shorten it to this :cmd.Parameters[DecimalValue].Value = (object) cdi.DecimalValue;cmd.Parameters[IntegerValue].Value = (object) cdi.IntegerValue;But wait, cmd.Parameters[DecimalValue].Value is of type object which means we have boxing going on here, this usually happens implicitly, the opposite is explicit, there is no need of the direct cast to object here.Shortening the codeTernary operatorYou can use the ternary operator here to shorten the if/else :if (parentContextName.IsNullOrEmpty()){    parentContextName += property.Name;}else{    parentContextName += . + property.Name;}Like this :parentContextName += parentContextName.IsNullOrEmpty() ? property.Name : . + property.Name;Count with predicateint totalcount = caseDataItems == null    ? -1    : caseDataItems.Where(c => !c.DataName.IsNullOrEmpty() && !c.DataType.IsNullOrEmpty()).Count();There is no point of first calling .Where() and than calling .Count() as both of those methods accept predicates which means you can reduce it to a single .Count() call like this :int totalcount = caseDataItems == null    ? -1    : caseDataItems.Count(c => !c.DataName.IsNullOrEmpty() && !c.DataType.IsNullOrEmpty());But you can shorten it even further with the null propagation + null coalescing operator like this :int totalcount = caseDataItems?.Count(c => !c.DataName.IsNullOrEmpty() && !c.DataType.IsNullOrEmpty()) ?? -1;string.Join(char separator, IEnumerable<T> value)You don't need the .ToArray() call here as string.Join() accepts IEnumerable<T> not strictly T[] :string[] parentContextParts = parentContextName.Split(new char[] {'.'},    StringSplitOptions.RemoveEmptyEntries);parentContextParts = parentContextParts.Take(parentContextParts.Length - 1).ToArray();parentContextName = string.Join(., parentContextParts);Can become :string[] parentContextParts = parentContextName.Split(new char[] {'.'},    StringSplitOptions.RemoveEmptyEntries);parentContextName = string.Join(., parentContextParts.Take(parentContextParts.Length - 1));Bonus tipGenerally, using .Take() to take all but the last element isn't recommended in my opinion as .Take() is suitable for all IEnumerable<T> but in order to obtain the length of a pure IEnumerable<T> you need to call .Count() which iterates the collection once and you need to iterate the collection once again to perform the O(n) operation .Take().It might be better to implement your own extension method which ensures that there is a single iteration over the IEnumerable<T> for example you might utilise the enumerator ?This doesn't concerns you however since you are using array and obtaining the length of array is O(1) operation while the the same operation on pure IEnumerable<T> would be O(n).SuggestionsYou can certainly use some generics here and there to avoid the casting.You should wrap TypeCode in a Dictionary combined with a simple Action resulting in Dictionary<TypeCode, Action>, so you can lookup the key and invoke the value in a single line, which is a lot more readable. But again generics will remove the need of all of those lines anyway.