Nice page.


I see several problems here.  


But first, the bug.  I believe you are getting the error because you are modifying the list by removing Page pi:

  Pages.remove(n);

which essentially changes the index numbers.  So if the page that it linked to has larger index number, the side-effect of that removal is that the index number is no longer relevant in the method's context for the pages with index number *higher* than n.  So defer that until after you decrement the link number of connected pages. Please try the bugfix and tell me if it works.

Second, I highly recommend using object-oriented approach to handling this data.  The problem arose because the Pages and Links are referred to using a common index number.  By using an iterator (this is included in ArrayList), you no longer need to juggle index numbers.   Iterator is extremely handy in case like your project.  

Lastly, the code can also benefit from encapsulation of connection numbers. I.e., instead of using c as a link counter, you can use:

class Page {
....
ArrayList links = ...


int getC(){
return links.size();
}

int removeLink(Link link){
 links.remove(links.indexOf(link);
}

}

above may be a cleaner approach.  It may even be better if you can have static links list in Link class, but you can't do that due to a slight quirk in Processing environment (you can't declare a static member in an inner class, I think that was the reason).


Please give it a shot!

P.S. As a side note, I recommend using the java naming conventions to help others read the code.  For example, Links should be "links", as it refers to an ArrayList instance instead of a class.



=======modified code==============


void kill(int n) {
 /* grab the page to be killed */
 Page pi=(Page) Pages.get(n);
 
 
 DPages.add(new DPage(new PVector(pi.x,pi.y), int(sqrt(pi.c))+6));
 
 //Pages.remove(n); // defer to end
 
 /* scan through links */
 for (int j=Links.size()-1; j>=0;j--) {
   /* grab link */
   Link l=(Link) Links.get(j);
   
   /* for the link */
   if (l.pa==n) {
     
     
     
     Page pc=(Page) Pages.get(l.pb);  //this line and the next don't work
      pc.c--;
     
     
      Links.remove(j);  //but this line does
   } else if (l.pb==n){
     
     Page pc=(Page) Pages.get(l.pa);  //these as well
      pc.c--;
   
   
      println("delete-link");
      Links.remove(j);
   }
   if (l.pa>=n) {
      l.pa--;
   }
   if (l.pb>=n) {
      l.pb--;
   }
   
   
   
 }
 
 Pages.remove(n); //<<<<< remove it here instead (sw01)
}

I see (no pun intended).

I'm a bit confused about here, so let me see what I see in code is correct:

you said:
======================

   Page pc=(Page) Pages.get(l.pb);  //this line and the next don't work
   pc.c--;

========================

so, the code are saying that:

===============
   pages is an arraylist:

   pages = [ .  .   .   .  .   .  pi .   .    .   .  .]
===============



Page pi = pages.get(n); // this needs to be removed


You use the variable pi to do some kind of routine:
=====================
   DPages.add(new DPage(new PVector(pi.x,pi.y), int(sqrt(pi.c))+6));
=====================

Links is an arraylist:

for any given link:

Link l

is like:


pa <---- l  -----> pb


***********
where pa and pb are NOT REALLY a page, but index number
***********

if you do this:

Pages.remove(n);

you are changing the index number in which the subsequent statement:

Page pc=(Page) Pages.get(l.pb);

works on.  l.pb may or may not be a correct index, depending on whether pb > n.  (btw, maybe the code should say pbIdx instead of pb.

This causes all types of trouble, because you are referring to a number that may or may not point to the correct object in the ArrayList.

so instead of:
---------------------
   class Link {
       int pa, pb;
       ...
   }
------------------

I recommend using
------------------
class Link {
   Page pa, pb;
   ...
}
-----------------


That's more oo.  

I can get into how to use an iterator, but I feel that it may not be appropriate in this forum since it's more to do with general programming than Processing.  But please check the pseudocode at the bottom of this reply for an idea of how simple the code becomes by using some of the oo approaches.


Anyway, back to the original problem: how do you know that the value c is not modified?

maybe you can try

println("#before##" + pc.c);
pc.c--;
println("#after##" + pc.c);


I bet it decreases.  Again please tell me why you think it's not decreasing.



##################################





void kill(Page pi){
 
 Dpages.add(new DPage(new PVector(pi.x,pi.y), int(sqrt(pi.getLinkCount()))+6));

 Iterator ite = Links.iterator();
 while(ite.hasNext()){
   Link lnk = (Link)ite.next();
   
   if(lnk.pa == pi){ // <-- note that it is comparing object("address"), and not integer.
     lnk.pb.removeLink(lnk);
     Links.remove(Links.indexOf(lnk));
   }else if(lnk.pb == pi){
     lnk.pa.removeLink(lnk);
     Links.remove(Links.indexOf(lnk));
   }
   
 }
 
 Pages.remove(n)
}

class Page{
 ArrayList links = new ArrayList();
 
 ...
 
 int getLinkCount(){
   return links.size();
 }
 
 void removeLink(Link lnk){
   links.remove(links.indexOf(lnk));
 }
}

class Link{
 Page pa, pb;
 ...
}

Wait, in which order did you execute these two procedures?

I believe I understand why you did what you did. I think the code changed two things at once, and they are related - it only exacerbates the indexing problem!

This code:



   if (l.pa>=n) {
l.pa--;
   }
   if (l.pb>=n) {
l.pb--;
   }



should be used only if the pa's index actually changes.  If this was used before

   if (l.pa==n) {
Page pc=(Page) Pages.get(l.pb);
pc.c--;
Links.remove(j);
   }  


, this will not correctly decrement the c value in the correct page.

This is because you've already shifted the index, so l.pa in the second code chunk (determined by l.pa = n) will not be correct ( because you've used (l.pa>=n) for index shift). That is because the first code chunk changed the definition of n in relation to the array.

Have you tried my suggestion to see if the pc.c is decremented during the procedure?  The suggestion and the method you use are not the same:  If you decrement the c value in the wrong page, you will not see the effect on the "page" node that the user is hovering over.

I totally understand your frustration on this matter.

I'm sorry if the explanation was confusing, but I really can't diagnose the bug without your  feedback specifically on the checks that I asked for - when you said you've tried everything, what do you mean by that?  Have you tried all of my suggestions?

I'm pretty much working in the dark here without your specific answer to my questions, and my effort here reduced to second guessing the results of these tests.  (It's probably faster for me to read the entire code by this time )

That said, I am guessing that your c value did not really decrement, because your "link-deleted" statement did not run at any point even if the link was removed. So,

=====================
Links.remove(j);
=====================

NEVER executed at any time (I am assuming that you did not do error catching here).

So I have a question here:

"On each iteration of the draw function i have a function to print both how many links and how many pages."

How did you implement the check function?

Please, please, please answer my questions instead of changing the topic!  I know this is your program and you know more about it than anyone else, but very often that familiarity prevents effective proofreading of the code.

Simple

Ok, here's what's going on:

In the main draw(),

 for(int i=Links.size()-1; i>=0;i--) {
   Link l=(Link) Links.get(i);
   if (l.dead) {
     Links.remove(i);  <=================HERE
   }
   else {
     l.force();
   }


You are removing the link before you process it in kill.

Just comment out that Links.remove(i); line above and your program will be in good shape.


Please give the object-oriented approach a try (see the example in the previous post).  It'll make the code a lot easier to understand!

No problem. Everybody goes blind if they stare into the terminal screen too long