I'm having trouble following your 'private T TypeConvert' method. Could you explain it a bit more or point me in the direction of a link I could read? I'm also considering posting a fourth question about Generics and/or Enums should we discuss it there?
–
Gabriel WMar 8 '14 at 6:54

I don't really like this. The public Get methods are perfectly capable of doing the null default and not-null conversion themselves, why should they have to write their own functionality as a lambda and inject it into another method that's now doing too much?
–
Ben AaronsonMar 8 '14 at 8:50

@dreza TypeConvert. It's not the end of the world or anything, it just seems that getting the property object and going through the type conversion logic are different enough to warrant their own methods.
–
Ben AaronsonMar 8 '14 at 8:56

Was going to upvote, but I don't agree with making WMI calls in a property getter... Ah, WTH, +1 for the rest of it ;)
–
Mat's MugMar 7 '14 at 16:54

@Mat'sMug I added that one on afterward. Really on the fence with regards to it. It feels property-ish, but yes, I agree WMI adds a layer of pitfalls that are contraindicative of the use of properties.
–
Jesse C. SlicerMar 7 '14 at 16:59

As for point 1, there can be reasons to enforce the existence of a default constructor, such as avoiding incompatibility when a non-default constructor is added later on - worth considering at least. As for the repeated pattern you noticed in 3, I'd be inclined to see if it could be converted to Get<T>([CallerMemberName] string property = null) or something of the sort.
–
MagusMar 7 '14 at 17:12

@Magus also, some serializers need a default constructor to work properly. But I would add a comment explaining that this was the reason for the empty constructor
–
Jeff VanzellaMar 7 '14 at 18:13

At this point, the most obvious candidate for refactoring seems to be your public methods on PropertyGetter, as they contain a lot of repeated logic. Note that for these examples I'm going to continue using the using statements from your posted coded, but from your last question I remember you were using these unnecessarily. You should only use using in this situation if the classes you're instantiating for moSearcher, collection and enu implements IDisposable. Otherwise all it does is make your code harder to read.

The simple refactoring would probably be as follows. Move the common logic to a private method, like:

The public methods will still all follow that common logic (get the property, if it's null return a default, otherwise convert to the desired format and return), but it's far more tolerable. The actual meat of the logic has been moved to its own method, which is exactly what you want for DRY.

Now, there's possible scope for further cutting down on the amount of code you have here using generics, but it depends a bit on taste whether you actually like this. The idea here would be to write a single public method:

public TProp Get<TProp>(string propertyName);

You may not like that name, feel free to change it, I'll keep using it for now. This would be fine if, for example, for null properties you returned default(T) and for non-null you returned (T)property. Unfortunately, you seem to have different ways of handling null and non-null results for your different types. This means you'd need to write something like:

Here, DefaultValue<T>() and Convert<T>(object property) would be methods that handled the conversion. Unfortunately this would mean a big ugly if or switch statement checking the type of T, which while not having unnecessary repetition, is hardly very nice either.

There is a way you could avoid this, and that's to have something like a 'PropertyConverter' class which handles the null and non-null conversion. You'd I could go into more detail on this, but at this point it seems to be adding a lot of complexity and really getting very little value out of it. My suggested solution would be to stick with the version in the first two chunks of code I posted.