views:

55

answers:

6

I am working on porting a VB6 application to C# (Winforms 3.5) and while doing so I'm trying to break up the functionality into various classes (ie database class, data validation class, string manipulation class).

Right now when I attempt to run the program in Debug mode the program pauses and then crashes with a StackOverFlowException. VS 2008 suggests a infinite recursion cause.

I have been trying to trace what might be causing this recursion and right now my only hypothesis is that class initializations (which I do in the header(?) of each class).

My thought is this:

  • mainForm initializes classA
  • classA initializes classB
  • classB initializes classA
  • ....

Does this make sense or should I be looking elsewhere?

UPDATE1 (a code sample):

mainForm

namespace john
{
    public partial class frmLogin : Form
    {
    stringCustom sc = new sc();

stringCustom

namespace john
{
   class stringCustom
   {
       retrieveValues rv = new retrieveValues();

retrieveValues

namespace john
{
    class retrieveValues
    {
    stringCustom sc = new stringCustom();
+3  A: 

Just put a break point in the constructor of each class you initialize. If you keep accessing the same breakpoints over and over again, I would say infinite recursion is the cause.

I would also check the stack to see what is going on.

Kevin
Even simpler: debug, and when StackOverFlowException pops out check the call stack - where the recursion is happening will be clear.
ANeves
Ok.. but how would you avoid it? =D
Nayan
Breakpoints definitely showed the suspected pattern.
John M
+1  A: 

Yeah, I think you are likely on the right track. You can sometimes see this easily in the debugger by looking at the call stack on a break point put at the line of code that causes the exception.

Tom Cabanski
+5  A: 

9 times out of 10, infinite recursion bugs are caused by bad property accessors:

public class BrokenClass
{
    private string name;

    public string Name
    {
        get { return name; }
        set { Name = value; }  // <--- Whoops
    }
}

I've also had it happen when doing major refactorings with method overloads; sometimes you accidentally end up with a method calling itself when it's supposed to call a different overloaded method.

Either way, you should be able to tell by looking at the call stack for the exception and checking for a repeating pattern. If you see one, then your problem is somewhere in that loop.

Edit - well, based on your example code, you definitely have infinite recursion in the initializers. I have no idea what that code is supposed to be doing, but it's never going to terminate. StringCustom immediately creates RetrieveValues which immediately creates another StringCustom, and so on.

This is one reason why circular class dependencies are typically considered a code smell. Whenever you see ClassA depending on ClassB and ClassB depending on ClassA then you should try to refactor; the exception is if ClassB is entirely owned and managed by ClassA (i.e. an inner class), which is clearly not the case here. You need to eliminate one of the dependencies somehow.

Aaronaught
+1: StringCustom > RetrieveValues > StringCustom > RetrieveValues > ...
ANeves
Thanks. It would appear my attempt to save code (by global class initialization) needs to re-thought.
John M
+1  A: 

Sound like that is the issue. Can you not pass ClassA into the constructor for ClassB?

Paul Manzotti
+2  A: 

Yes, you have an infinite recursion going on because you have two classes which create an instance of the other class in their constructors. As soon as you create an instance of one class, it creates an instance of the other class, which creates an instance of the other class, which creates an instance of the other class etc. etc. etc.

You definitely need to refactor this.

MusiGenesis
+1  A: 

Well, everybody understands it. Why not suggest some solution then?

Now, I remember this situation. One way is to avoid calling another contructor inside one. So, there would be extra coding. Eg -

class A {
    B b;
    A() {}
    void Init() { b = new B(); }
}
class B {
    A a;
    B() {}
    void Init() { a = new A(); }
}
...
A aObj = new A();
aObj.Init();
...
B bObj = new B();
bObj.Init();

This will remove the recursion. This is, obviously, easiest way. :)

Nayan
thanks for the practical example.
John M
You're welcome. :)
Nayan