C# question. Attempting to simplify some code

KoihimeNakamura said:
Here is the code:
http://pastebin.com/0ZL8rJb1

The main thing I want to see if I can simplify is the validation functions and the LoadProperties function. It's very repetitive, so I'm curious if I'm writing it wrong. The json file, PropertyName and PropertyHandler are added to show where the code is coming from.
Click to expand...

Most people would use a switch statement instead of all the ifs. I prefer to use a specification pattern or something similar in this instance (a class to handle each value of "thisProperty" with a "CanHandle(property)" method).
 
KoihimeNakamura said:
Here is the code:
http://pastebin.com/0ZL8rJb1

The main thing I want to see if I can simplify is the validation functions and the LoadProperties function. It's very repetitive, so I'm curious if I'm writing it wrong. The json file, PropertyName and PropertyHandler are added to show where the code is coming from.
Click to expand...

There are lots of ways of refactoring it - whether it's simpler is a matter of opinion.

Since it looks like all of the validation is simple range checking, I would create a validator interface and a generic validation method which will validate any IValidator interface. Then create an object for each of your individual validations. Something like the below.


Code: 
        public bool Validate<T>(T itemToValidate) where T : IValidator
        {
            if (itemToValidate.Value < itemToValidate.Min && itemToValidate.Value > itemToValidate.Max)
            {
                return false;
            }
            return true;
        }

        public interface IValidator
        {
            int Value { get; set; }
            int Min { get; }
            int Max { get; }
        }

Also, that vast set of else-if's MUST GO. Replace with a switch statement instead.
 
Last edited:
Newtons Bit said:
Also: you can use attributes in enums, and use those attributes to define things like mins and maxes:

http://pastebin.com/Jhr9kT6Q
Click to expand...

I actually tried to do this originally, but I incorrectly read the AttributeTargets documentation as suggesting that attributes could only decorate Enum declaration, not the Enum fields themselves.

Your solution is much better than mine, makes for much nicer looking code ^_^
 
KoihimeNakamura said:
While that is pretty helpful, some of the global parameters are bools and strings. I'm not sure how to generically parse that.
Click to expand...

You could put the type into the attribute, store the parameter as an object in the dictionary and use the attribute to cast it back into the correct type.

It would be very similar to how you did it here:
https://gist.github.com/anonymous/d65a9fd08ae425ff8157

The attribute could have a minF/maxF property which would be used for floats, minI/maxI for ints, something different for bool testing or string testing that would become relevant based on what Type you set in the constructor.
 
So it turns out if you do this:

Newtonsoft.Json.Linq.JValue testVal = new Newtonsoft.Json.Linq.JValue(100);
int iTVal = (int)testVal;

That works.

But if you put that same thing into an Dictionary<enum, object> it fails the cast every time. And since I'm using that as my json reader, I kinda need to figure out a way around that.

ETA: Okay, I found a way. Convert.ToInt32 - just need to work on it triggering inappropriately on doubles.
 
Last edited:
You must log in or register to reply here.

Back
Top Bottom