Static Methods vs Class Instances and return values in C#

Question

I have various classes for handling form data and querying a database. I need some advice on reducing the amount of code I write from site to site.

The following code is for handling a form posted via ajax to the server. It simply instantiates a Form class, validates the data and processes any errors:

public static string submit(Dictionary<string, string> d){

    Form f = new Form("myform");

    if (!f.validate(d)){

        return f.errors.toJSON();

    }

    //process form...

}

Is there a way to reduce this down to 1 line as follows:

if (!Form.validate("myform", d)){ return Form.errors.toJSON(); }

Answer 1

I would move all common validation logic to a superclass.

I think the main problem of your code is not that is long, but that you're repeating that in many places, either if you manage to make it a one-liner, it would not be DRY.

Take a look at the Template Method pattern, it might help here (The abstract class with the validation would be the Template and your specific 'actions' would be the subclasses).

Answer 2

Of course you could write this:

public static string FormValidate(Dictionary<string, string> d)
{
    Form f = new Form("myform");
    if (!f.validate(d))
        return f.errors.ToJSON();

    return null;
}

then your submit can be:

public static string submit(Dictionary<string, string> d)
{
    if ((string errs = FormValidate(d))!= null) { return errs; }
    // process form
}

That cuts down your code and doesn't hurt readability much at all.

Answer 3

Let's break that down into two questions.

1) Can I write the existing logic all in one statement?

The local variable has to be declared in its own statement, but the initializer doesn't have to be there. It's prefectly legal to say:

Form f; 
if (!(f=new Form("myform")).validate(d))return f.errors.toJSON(); 

Why you would want to is beyond me; doing so is ugly, hard to debug, hard to understand, and hard to maintain. But it's perfectly legal.

2) Can I make this instance method into a static method?

Probably not directly. Suppose you had two callers validating stuff on two different threads, both calling the static Form.Validate method, and both producing errors. Now you have a race. One of them is going to win and fill in Form.Errors. And now you have two threads reporting the same set of errors, but the errors are wrong for one of them.

The better way to make this into a static method is to make the whole thing into a static method that has the desired semantics, as in plinth's answer.

Errors errors = Validator.Validate(d);
if (errors != null) return errors.toJSON();

Now the code is very clear, and the implementation of Validate is straightforward. Create a form, call the validator, either return null or the errors.

I would suggest that you don't need advice on reducing the amount of code you write. Rather, get advice on how to make the code read more like the meaning it intends to represent. Sometimes that means writing slightly more code, but that code is clear and easy to understand.

Answer 4

If you really, really wanted to, you could store the error text in a thread-local property.

http://stackoverflow.com/questions/2158981/does-c-have-a-threadlocal-analog-for-data-members-to-the-threadstatic-attr