methods in constructors, bad?

I have a windows form, and I have a class 开发者_如何学编程that checks a text file to make sure it has certain aspect. Now I have methods in a constructor and it seems a little weird. Should I just leave the constructor blank, and implement a start() type method and call that? So far my code looks like this

public class Seating
{
    private int segments = 0;
    public Seating()
    {
        checkInvoice();
        getSegmentCount();          
    }
}

I have methods in a constructor and it seems a little weird

Well, why else would a constructor exist? It’s entirely legitimate to have methods in a constructor, even methods that could fail (thus interrupting the object creation). It’s hard to say whether this is appropriate in your particular case.

In general it’s fine though.

There's nothing wrong with calling non-virtual methods in the constructor. By the time your constructor fires, your parent object has been fully constructed. However, you do not want to call any virtual methods, as they could be overridden by subclasses, which would execute code within them when they are not fully constructed.

The general rule is have methods that are very trivial and unlikely to throw exceptions in constructor. If the methods might fail for any reason (file access or missing, DB issue, null reference ...), then it should not be in the constructor, as people should expect constructors not to fail (especially parameterless constructors, although the general guidance doesn't specify). People should not expect var seating = new Seating(); to be source of errors.

You can add Start() / Initialize() kind of methods as instance methods as you mentioned. You can also add a new static method that returns a new instance of the Class after calling the two methods you need on it and make the constructor private. You can go further and have this method in a new Factory class (making the constructor internal). You can think of other ways to do it also.

One thing is you may see any value calculated or retrieved in these methods a constructor parameter (and have no parameterless constructor), then the constructor will only assign those parameters to respective fields/properties. A factory method or service method in the same assembly or other "Services" dedicated assembly can be responsible for calling the methods, getting the parameters, passing them to the constructor, and returning a new instance of the class. This is my personal favorite.

Generally speaking this kinds of problems is a sign that the class is doing too much and you might want to split the functionality into other classes (existing or new). That's why the last solution is suggested, so that the two methods themselves might be in the other "Services" assembly, but as mentioned above, there are many other ways to do it if you want.

Update:

Here is Microsoft Guidelines for Constructors:
Constructor Usage Guidelines

Quoting from the page:

Minimize the amount of work done in the constructor. Constructors should not do more than capture the constructor parameter or parameters. This delays the cost of performing further operations until the user uses a specific feature of the instance.

Update 2

The above page moved to a living document (which means it's updatable) under the name Constructor Design.

Generally speaking, your constructor should only be setting up the state of the object. It is possible that could mean calling methods. I would say that running a very long methods is not a good idea. The user of your class wouldn't expect the constructor to be an expensive method.

I don't you didn't mention virtual methods, but you should NEVER call these in a constructor.

Here is an example of how it can get back quickly:

public class MyBase
{
        protected MyBase()
        {
                this.VirtualMethod();
        }

        protected virtual void VirtualMethod()
        {
                Console.WriteLine("VirtualMethod in MyBase");
        }
}

public class MyDerived : MyBase
{
        private readonly string message = "Set by initializer";

        public MyDerived(string message)
        {
                this.message = message;
        }

        protected override void VirtualMethod()
        {
                Console.WriteLine(this.message);
        }
}

Now, lets say you have this code elsewhere:

MyDerived d = new MyDerived("Called from constructor");

What do you think will be shown on the console? If you said "Set by initializer", then you are correct.

This is why:

• All of the field initializers are executed before the code in the constructor.
• The C# compiler adds a call to the base constructor before anything that is user defined. In this case, it calls up to
  MyBase which calls the VirtualMethod(). Since the runtime type of d is MyDerived, the override of VirtualMethod() in MyDerived is
  executed. And now since the body of MyDerived's constructor hasn't
  executed yet, this.message has the value it was given in the
  initalizer.
• Now the body of MyDerived's constructor is executed.
• Later calls to VirtualMethod() on this instance will now print out
  "Called from constructor".

Virtual method calls in the constructor are no-go (with the tiny exception of sealed classes, which makes the method effectively non-virtual).

Non-virtual methods can be okay, but you should make sure that all members have been initialized before, and the non-virtual methods may not call any virtual member as well.

There are some useful design guidelines for constructor usage here:

Constructor Design (MSDN)

The text is lifted from:

Framework Design Guidelines: Conventions, Idioms, and Patterns for Reusable .NET Libraries

Which is a worthy purchase.

The one I got bitten by years ago was: "Do not call virtual members on an object inside its constructors."

You’re free to put as much code in there as you like, but by convention, developers usually expect the constructor to do very little — its main job is to ensure that the object is in a valid initial state. That might involve checking the arguments and throwing an exception if there’s a problem, but not much else. You are likely to surprise developers who use your class if you write a constructor that does something nontrivial, such as adding data to a database or sending a message over the network