Stock Overflow Error

[Alex Mun]

Hi,

I have the following code:

import java.io.*;

class PilaEnlazada
{
private int elemento;
private PilaEnlazada top,aux,fin;
PilaEnlazada n=new PilaEnlazada();

public PilaEnlazada()
{
n.elemento=0;
n.top=null;
n.fin=null;
n.aux=null;
}

public void push(int e)
{
PilaEnlazada temp=new PilaEnlazada();

temp.elemento=e;
if (n.top==null)
{
n.top=n;
n.aux=n;
}
if (n.top!=null)
{
temp.elemento=e;
temp.fin=top;
top=temp;
}
}

public static void main(String args[])
{
PilaEnlazada nuevo=new PilaEnlazada();
nuevo.push(1);
}
}

When I compile this lines there is no error. But when I run it I obtain the following error:

java.lang.StackOverflowError at PilaEnlazada<init>(PilaEnlazada.java:7)

I don't understand why it appears this error because I only have one number and any bucle.

Any ideas?

Thanks.

[Stan James]

Stack overflows are often due to recursion gone wild, so I looked for anything that referes to itself. We usually see a method call itself, but you found an interesting way to do it with object references. Look at your member variable definition:

PilaEnlazada n=new PilaEnlazada();

When your main() method creates a new instance of PilaEnlazada, this variable is initialized, and it creates another instance. And that new one creates another instance. As my wife always says about children's dirty laundry, it just never ends.

Describe your objective to us a bit in plain language (not code) and see if a better way to do it in code comes to mind.

[jiju ka]

Corrected code:

import java.io.*;
 
class PilaEnlazada
{
private int elemento;
private PilaEnlazada top,aux,fin;
 
public PilaEnlazada() 
{
elemento=0;
top=null;
fin=null;
aux=null;
}

public void push(int e) 
{
PilaEnlazada temp=new PilaEnlazada();

temp.elemento=e;
if (top==null) 
{
top=this;
aux=this;
}
if (top!=null) 
{
temp.elemento=e;
temp.fin=top;
top=temp;
}
}
 
public static void main(String args[])
{
PilaEnlazada nuevo=new PilaEnlazada();
nuevo.push(1);
}
}

Problems found:
The class variable 'n' and it's initialization

PilaEnlazada n=new PilaEnlazada();

From the main when
PilaEnlazada nuevo=new PilaEnlazada();
is executed a recursion is started.

Below I will put the path of the recursion...

step 1. When new PilaEnlazada() is executed the class PilaEnzalada is loaded to memmory. If already loadecd ignore this step.
Step 2. JVM allots memory for all class variables.
Step 3. All class variables including 'n' will be initialized before executing the statements in constructor.
Step 4. As part of intializing n, the statement new PilaEnlazada() will be executed
Step 5. As part of step 4 above Steps 1 to 5 will be executed again.

Solution to the program:
(solution to the program is different from solution to the problem, I will explain the problem later)Remove recursion caused by class variable 'n'. Remove class variable 'n' and all it's references. The 'current' PilaEnlazada can be refrenced by 'this'. On the left hand side of statements, 'this' is implicit even if you don't specify them.
ex:

public PilaEnlazada() 
{
elemento=0;
top=null;
fin=null;
aux=null;
}
can also be written as
public PilaEnlazada() 
{
this.elemento=0;
this.top=null;
this.fin=null;
this.aux=null;
}
 

On the right hand side of statements, 'this' is explicitly needed.

if (top==null) 
{
top=n;
aux=n;
}
should be converted to 
 
if (top==null) 
{
top=this;
aux=this;
}

Solution to the problem:
Solution is specific to the goal.
Goal identification:
What the programmer trying to accomplish here is either to establish a linked list of 'PilaEnlazada's or a collection of 'PilaEnlazada's.
Since PilaEnlazada itself have a behaviour called push it is assumed that the programmer is trying to impliment a 'stack kind of collection' for 'PilaEnlazada's. If the goal is different from assumed please let me know.

Solution:
Needs atleast two types need to be defined. One type is PilaEnlazada. Another type is a 'stack kind of collection' which holds PilaEnlazada.
The code for PilaEnlazada may look like.

class PilaEnlazada
{
public PilaEnlazada() 
{
}
}

The code for PilaEnlazadaStack may look like follows

class PilaEnlazadaStack
{
        private static PilaEnlazadaHolder top;
        private static PilaEnlazadaHolder current;
 
public PilaEnlazadaStack() 
{

}

public void push(PilaEnlazada current) 
{
PilaEnlazadaHolder peCurr = new PilaEnlazadaHolder(current);
if (top==null) 
{
top = peCurr;
                        current = peCurr;
}
else //if (top!=null) 
{
                        peCurr.prev = current;
                        current.next = peCurr;
                        peCurr.elemento = current.elemento + 1;
                        current = peCurr;
                        top = peCurr;
}
}
 
public static void main(String args[])
{
                PilaEnlazadaStack peStack =new PilaEnlazadaStack();
 
PilaEnlazada nuevo=new PilaEnlazada();
peStack.push(nuevo);
}
}
 
class PilaEnlazadaHolder
{
     public PilaEnlazadaHolder prev,next;
     public PilaEnlazada pe; 
     public int elemento;
     PilaEnlazadaHolder(PilaEnlazada pe)
     {
        this.pe = pe;
elemento=0;//need not set; null and zero are default values
prev=null;// ditto
next=null;//ditto
     }
}

PilaEnlazadaHolder can be made as an inner class to PilaEnlazadaStack

Another solution is to use the Stack
http://java.sun.com/j2se/1.4.2/docs/api/java/util/Stack.html

but it extends Vector which could cause performance issues with synchronization