Redis Object storage and conversion

Question

I am stepping into a new realm with this project, Reflection. I have written some working code (below) that will potentially store POCO objects in a redis cache (and eventually backed by Table or other nosql persistant storage).

Currently it only reads the objects from the cache, but after this review I will be attempting to reverse the process and store the POCOs in the cache.

I am posting this due to my weakness with Reflection, but feel free to correct me on any other issues.

using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.Linq;
using Demgel.Redis.Attributes;
using StackExchange.Redis;
namespace Demgel.Redis
{
 public static class DemgelRedis
 {
 public static IEnumerable<HashEntry> ConvertToRedisHash(object o, bool ignoreFail = false)
 {
 var hashList = new List<HashEntry>();
 foreach (var prop in o.GetType().GetProperties())
 {
 var type = prop.PropertyType;
 if (type.IsAssignableFrom(typeof (Guid)))
 {
 var guid = prop.GetValue(o, null) as Guid?;
 if (guid.HasValue)
 {
 hashList.Add(new HashEntry(prop.Name, guid.Value.ToString()));
 }
 }
 else if (type.IsAssignableFrom(typeof(string)))
 {
 var value = prop.GetValue(o, null) as string;
 if (value != null)
 {
 hashList.Add(new HashEntry(prop.Name, value));
 }
 }
 }
 return hashList;
 }
 public static object ConvertToObject(object obj, HashEntry[] hashEntries, bool ignoreFail = false)
 {
 var hashDict = hashEntries.ToDictionary();
 foreach (var prop in obj.GetType().GetProperties())
 {
 RedisValue hashPair;
 if (!hashDict.TryGetValue(prop.Name, out hashPair)) continue;
 var type = prop.PropertyType;
 if (type.IsAssignableFrom(typeof(Guid)))
 {
 Guid value;
 if (!Guid.TryParse(hashPair, out value))
 {
 try
 {
 value = new Guid((byte[]) hashPair);
 }
 catch
 {
 value = Guid.Empty;
 }
 }
 prop.SetValue(obj, value);
 }
 else if (type.IsAssignableFrom(typeof(string)))
 {
 prop.SetValue(obj, (string)hashPair);
 }
 else if (type.IsAssignableFrom(typeof(float)))
 {
 prop.SetValue(obj, (float)Convert.ToDouble(hashPair));
 }
 else if (type.IsAssignableFrom(typeof(double)))
 {
 prop.SetValue(obj, Convert.ToDouble(hashPair));
 }
 else if (type.IsAssignableFrom(typeof(int)))
 {
 prop.SetValue(obj, int.Parse(hashPair));
 }
 else if (type.IsAssignableFrom(typeof (DateTime)))
 {
 DateTime dateTime;
 if (DateTime.TryParse(hashPair, out dateTime))
 {
 prop.SetValue(obj, dateTime);
 }
 }
 }
 return obj;
 }
 public static DemgelRedisResult<T> RetreiveObject<T>(string id, IDatabase redisDatabase)
 where T : class, new()
 {
 var ret = RetreiveObject(new T(), id, redisDatabase);
 // Now try to put it all back together
 var result = ret.Object as T;
 var redisResult = new DemgelRedisResult<T>
 {
 Object = result,
 Result = (result == null) ? DemgelResult.NotFound : ret.Result
 };
 return redisResult;
 }
 /// <summary>
 /// Retrieves an object from redis cache by id
 /// 
 /// usually assumes a key:id structure, but if no key field is supplied
 /// will just search by id
 /// 
 /// can use key:id:suffix
 /// </summary>
 /// <param name="obj"></param>
 /// <param name="id">The id of the object to find</param>
 /// <param name="redisDatabase"></param>
 /// <returns></returns>
 private static DemgelRedisResult RetreiveObject(object obj, string id, IDatabase redisDatabase)
 {
 // We need to build the key
 var classAttr = obj.GetType().GetCustomAttributes(true);
 string prefix = null, suffix = null, redisKey;
 foreach (var attr in classAttr)
 {
 if (attr is RedisPrefix)
 {
 prefix = ((RedisPrefix)attr).Key;
 Debug.WriteLine("Key Found");
 } else if (attr is RedisSuffix)
 {
 suffix = ((RedisSuffix) attr).Key;
 Debug.WriteLine("Suffix Found.");
 }
 }
 if (prefix != null)
 {
 redisKey = suffix != null ? $"{prefix}:{id}:{suffix}" : $"{prefix}:{id}";
 }
 else
 {
 redisKey = suffix != null ? $"{id}:{suffix}" : id;
 }
 Debug.WriteLine($"{redisKey}");
 var ret = redisDatabase.HashGetAll(redisKey);
 var result = new DemgelRedisResult
 {
 Result = DemgelResult.Success
 };
 if (ret.Length == 0)
 {
 result.Result = DemgelResult.NotFound;
 return result;
 }
 // Attempt to set all given properties
 result.Object = ConvertToObject(obj, ret, true);
 var props = result.Object.GetType().GetProperties();
 foreach (var prop in props)
 {
 if (prop.CustomAttributes.Any(x => x.AttributeType == typeof (RedisIdKey)))
 {
 Debug.WriteLine("RedisIdKey was found.");
 if (!prop.PropertyType.IsAssignableFrom(typeof (string)))
 {
 throw new InvalidOperationException("Id can only be of type String");
 }
 prop.SetValue(result.Object, id);
 }
 else
 {
 // If value is not set, then recursion
 if (prop.GetValue(result.Object) != null) continue;
 var a = Activator.CreateInstance(prop.PropertyType);
 var subresult = RetreiveObject(a, id, redisDatabase);
 if (subresult.IsValid)
 {
 prop.SetValue(result.Object, subresult.Object);
 }
 }
 }
 return result;
 }
 }
}

Answer 1

If I focus on how ConvertToRedisHash is returning its IEnumerable<HashEntry>, I get this:

public static IEnumerable<HashEntry> ConvertToRedisHash(object o, bool ignoreFail = false)
{
 var hashList = new List<HashEntry>();
 foreach (var prop in o.GetType().GetProperties())
 {
 ...
 hashList.Add(new HashEntry(prop.Name, guid.Value.ToString()));
 ...
 hashList.Add(new HashEntry(prop.Name, value));
 ...
 }
 return hashList;
}

You're returning an IEnumerable, but you're not returning anything until you've collected every item and added them to a List. You could instead drop that List, and yield return the results as they become available:

public static IEnumerable<HashEntry> ConvertToRedisHash(object o, bool ignoreFail = false)
{
 foreach (var prop in o.GetType().GetProperties())
 {
 ...
 yield return new HashEntry(prop.Name, guid.Value.ToString());
 ...
 yield return new HashEntry(prop.Name, value);
 ...
 }
}

---

There's a typo with RetreiveObject, should be RetrieveObject ;-)

---

I don't like this part:

var type = prop.PropertyType;
if (type.IsAssignableFrom(typeof(Guid)))
{
 Guid value;
 if (!Guid.TryParse(hashPair, out value))
 {
 try
 {
 value = new Guid((byte[]) hashPair);
 }
 catch
 {
 value = Guid.Empty;
 }
 }
 prop.SetValue(obj, value);
}
else if (type.IsAssignableFrom(typeof(string)))
{
 prop.SetValue(obj, (string)hashPair);
}
else if (type.IsAssignableFrom(typeof(float)))
{
 prop.SetValue(obj, (float)Convert.ToDouble(hashPair));
}
else if (type.IsAssignableFrom(typeof(double)))
{
 prop.SetValue(obj, Convert.ToDouble(hashPair));
}
else if (type.IsAssignableFrom(typeof(int)))
{
 prop.SetValue(obj, int.Parse(hashPair));
}
else if (type.IsAssignableFrom(typeof (DateTime)))
{
 DateTime dateTime;
 if (DateTime.TryParse(hashPair, out dateTime))
 {
 prop.SetValue(obj, dateTime);
 }
}

It seems it would be better off as a switch block.