Zen and the Art of Code Sculpturing

I find myself in the state of mindfulness... I am aware of myself, the code, but nothing in the surrounding... I move the lines around just a bit faster than I type... I shape the code... I am the sculptor... I aim for simplicity, this implies correctness and beauty... And I can see how out of small moves, symbol, little bits the program evolves, it takes on definitive form, its own character... There are many different methods and techniques that people created for developing code. Object-oriented programming, Design Patterns, Refactoring, Iterative development process, are all very useful and much needed techniques to produce good systems. While being useful recipes, they underemphasize two important points: programmers want to be creative complex systems can not be build, but must evolve I don't want to sit down and think which design pattern or which refactoring I need to apply. I am not a machine, I want to enjoy the experience of programming. Instead, I want to recognize that I am applying a design pattern or refactoring WHEN I am doing it. The principles that I employ are simpler and more vague than the ones prescribed in most books: Axiom of Simplicity Is the thing I am looking at simple? (if not, change it) Can I make it simpler? (if not, I am done) Axiom of Correctness Does it do what I need it to do? (if not, add more) Does it do something else? (if yes, take it out and if in doubt, throw it out.) Axiom of Time To the best of my knowledge, today, did I do my best? (if yes, I am done) Can I make it better today than yesterday? (if yes, change everything) To me these principles are natural consequences of Complexity Science, my personal experience and body of work in software engineering. Each of you should have their own methodology. It almost does not matter what it is as long as it is consistent and usable. In reality, when you come to the subject as complex as modern software engineering, there is a big chance that you will be confused. Its okay. It is normal. With time you will get unconfused. For now, you will probably need to be more conscience about using one of the standard methodologies for developing programs. If you get lucky and start project from scratch, you should start with regular OOP design. Identify critical classes in your system and relationships between. Create various diagrams. Think of design patterns. As project unfolds, and departs from the original design. Use refactoring, to harness the system complexity. If you walk into an ongoing project, you will likely to need refactoring right way. Refactoring is the technique of continuously improving existing code structure, without changing its functionality. Refactoring became very popular with when Martin Fowler wrote his now famous book. Refactoring is based on the following essential principles: Unless you cleanup with time things get ugly Ugly code ALWAYS bytes There is a better way There is a simpler way/li> Bad code smells Writing good code is easy Rewriting from scratch is error-prone Just like design patterns are recipes for good design, Refactoring has techniques for changing the existing code. The most important thing emphasized by refactoring is testing. Refactoring is focused on changing the code WITHOUT changing its functionality. To ensure that, programmer run (very) frequent tests on the code as they refactor. (Admittedly, I do not do that often enough... Partially out of arrogance and partially because I have not figured out a good way to test complex applications...). The JUnit framework developed by widely recognized agile methods guru, Kent Beck, is used by the refactoring community. The framework is introduces simple ideas of Test and TestCase and allows you to write and run tests for your applications. Before JUnit, you could just write main in every one on your classes and test. We are not going to have time to go over all of the refactoring techniques and recipes. I would point you to Refactoring home page and a tutorials Refactoring basics Alright, enough talking lets get to it! Here is an extract from last year BasicSite main loop (by anonymous group). public void run() { // Object neighbors[] = new Object[4]; // used to check against bottom-left, bottom, bottom-right, right neighbors, int numberOfNeighbors = 4; Object neighbors[] = new Object[ numberOfNeighbors ]; // use a random number to decide which one to interact with ArrayList neighborNumbers = new ArrayList(); initPositions(); // make sure site grid is initialized populateSiteGrid(); // now fill in the entities and resources // setup the listener, giving the renderer a statisticsManager initSiteListeners(); // we have liftoff bResume = true; bRunning = true; while ( bRunning ) { if ( bResume ) { //System.out.println( "Entities# = " + numberOfEntities() + ", Resources# = " + numberOfResources() ); // move then interact so it will be clearer on the renderer which entity interacted with who moveEntities(); // the resources regrow or dies out renewResources(); removeDepletedResources(); // have entites eat resourceFeeder(); // clone entities cloneEntities(); // reset for next round of interactions for ( Iterator e = entities(); e.hasNext(); ) ( (IEntity) e.next() ).reset(); for ( Iterator iter = entities(); iter.hasNext(); ){ IEntity entity = (IEntity) iter.next(); int xPos = entity.getX(); int yPos = entity.getY(); // check for all neighbors, but only interact with one neighbors[ 0 ] = siteGrid[ xPos + 1 ][ yPos + 0 ]; neighbors[ 1 ] = siteGrid[ xPos + -1 ][ yPos + 1 ]; neighbors[ 2 ] = siteGrid[ xPos + 0 ][ yPos + 1 ]; neighbors[ 3 ] = siteGrid[ xPos + 1 ][ yPos + 1 ]; // neighbors[ 4 ] = siteGrid[ xPos + -1 ][ yPos + -1 ]; // neighbors[ 5 ] = siteGrid[ xPos + 0 ][ yPos + -1 ]; // neighbors[ 6 ] = siteGrid[ xPos + 1 ][ yPos + -1 ]; // neighbors[ 7 ] = siteGrid[ xPos + -1 ][ yPos + 0 ]; //get the entities right, bottom-left bottom, and bottom-right neighbor // for( int i = 0; i < 3; i++ ){ // neighbors[ i ] = siteGrid[ xPos + ((i + 1) / 2)][ yPos + ((i+1) % 2 )]; // }// for loop // used for picking a random neighbor to interact with for ( int i = 0; i < numberOfNeighbors; i++ ) neighborNumbers.add( new Integer( i ).toString() ); // while( !entity.isInteracted() && neighborNumbers.size() > 0 ) { while( neighborNumbers.size() > 0 ) { //if there was something next to the right, bottom, bottom-left //of bottom-right of the entity int i = Integer.parseInt( (String) neighborNumbers.remove( new Double( Math.random() * neighborNumbers.size() ).intValue() ) ); if( neighbors[ i ] != null ){ //check if the neighbor is another entity //as opposed to a resource if( neighbors[ i ] instanceof IEntity ) { IEntity neighbor = (IEntity) neighbors[ i ]; if ( entity.isAlive() && neighbor.isAlive() && !neighbor.isInteracted() ) { interact( entity, neighbor); // entity.interacted(); // neighbor.interacted(); } } } // first if } } removeOldDeadEntitiesRemains(); checkMortality(); removeDeadEntities(); addChildEntities(); // entities age incrementAge(); // notify site listeners of changes if ( siteRenderer != null ) { siteRenderer.SiteChanged(); statisticsManager.SiteChanged(); } // slow down simulation if the user is viewing it try { if ( siteRenderer != null && siteRenderer.isSiteViewable() ) { Thread.sleep( DELAY_CONST ); } } catch( Exception e ) { Log.error( "Error in sleeping" ); } } else { try { Thread.sleep( QUERY_CONST ); } catch( Exception e ) { Log.error( "Error in sleeping" ); } } } } How does this look? Not so bad right?! Not horrible, but it can be cleaned up a bit. The trick is to refactor the code bit by bit, without chaning functionality or introducing a bug. It doesn't matter where you start, start anywhere and move arond, things will fall into place. I like to start by removing things that are not used, for example comments that don't add any value: public void run() { // Object neighbors[] = new Object[4]; // used to check against bottom-left, bottom, bottom-right, right neighbors, int numberOfNeighbors = 4; Object neighbors[] = new Object[ numberOfNeighbors ]; What does this mean?... Flag and hold off... // use a random number to decide which one to interact with ArrayList neighborNumbers = new ArrayList(); initPositions(); Not useful -> out// make sure site grid is initialized populateSiteGrid(); Not useful -> out// now fill in the entities and resources Not useful -> out// setup the listener, giving the renderer a statisticsManager initSiteListeners(); What does this mean?... Flag and hold off...// we have liftoff bResume = true; bRunning = true; while ( bRunning ) { if ( bResume ) { Debugging -> out//System.out.println( "Entities# = " + numberOfEntities() + ", Resources# = " + numberOfResources() ); Not useful -> out// move then interact so it will be clearer on the renderer which entity interacted with who moveEntities(); What does this mean?... Flag and hold off...// the resources regrow or dies out renewResources(); removeDepletedResources(); Comment and method name mismatch - rename method, remove comment//// have entites eat resourceFeeder(); Not useful -> out//// clone entities cloneEntities(); What does this mean?... Flag and hold off...// reset for next round of interactions for ( Iterator e = entities(); e.hasNext(); ) ( (IEntity) e.next() ).reset(); for ( Iterator iter = entities(); iter.hasNext(); ){ IEntity entity = (IEntity) iter.next(); int xPos = entity.getX(); int yPos = entity.getY(); // check for all neighbors, but only interact with one neighbors[ 0 ] = siteGrid[ xPos + 1 ][ yPos + 0 ]; neighbors[ 1 ] = siteGrid[ xPos + -1 ][ yPos + 1 ]; neighbors[ 2 ] = siteGrid[ xPos + 0 ][ yPos + 1 ]; neighbors[ 3 ] = siteGrid[ xPos + 1 ][ yPos + 1 ]; // neighbors[ 4 ] = siteGrid[ xPos + -1 ][ yPos + -1 ]; // neighbors[ 5 ] = siteGrid[ xPos + 0 ][ yPos + -1 ]; // neighbors[ 6 ] = siteGrid[ xPos + 1 ][ yPos + -1 ]; // neighbors[ 7 ] = siteGrid[ xPos + -1 ][ yPos + 0 ]; What does this mean?... Flag and hold off... //get the entities right, bottom-left bottom, and bottom-right neighbor // for( int i = 0; i < 3; i++ ){ // neighbors[ i ] = siteGrid[ xPos + ((i + 1) / 2)][ yPos + ((i+1) % 2 )]; // }// for loop What does this mean?... Flag and hold off...// used for picking a random neighbor to interact with for ( int i = 0; i < numberOfNeighbors; i++ ) neighborNumbers.add( new Integer( i ).toString() ); Old -> remove // while( !entity.isInteracted() && neighborNumbers.size() > 0 ) { while( neighborNumbers.size() > 0 ) { Comment does not correspond to the code... Flag, revisit //if there was something next to the right, bottom, bottom-left //of bottom-right of the entity int i = Integer.parseInt( (String) neighborNumbers.remove( new Double( Math.random() * neighborNumbers.size() ).intValue() ) ); if( neighbors[ i ] != null ){ Not useful -> out //check if the neighbor is another entity //as opposed to a resource if( neighbors[ i ] instanceof IEntity ) { IEntity neighbor = (IEntity) neighbors[ i ]; if ( entity.isAlive() && neighbor.isAlive() && !neighbor.isInteracted() ) { interact( entity, neighbor); Old code -> out // entity.interacted(); // neighbor.interacted(); } } } // first if } } removeOldDeadEntitiesRemains(); checkMortality(); removeDeadEntities(); addChildEntities(); Not useful -> out // entities age incrementAge(); Not useful -> out // notify site listeners of changes if ( siteRenderer != null ) { siteRenderer.SiteChanged(); statisticsManager.SiteChanged(); } // slow down simulation if the user is viewing it try { if ( siteRenderer != null && siteRenderer.isSiteViewable() ) { Thread.sleep( DELAY_CONST ); } } catch( Exception e ) { Log.error( "Error in sleeping" ); } } else { try { Thread.sleep( QUERY_CONST ); } catch( Exception e ) { Log.error( "Error in sleeping" ); } } } } Now really take them out: public void run() { // Object neighbors[] = new Object[4]; // used to check against bottom-left, bottom, bottom-right, right neighbors, int numberOfNeighbors = 4; Object neighbors[] = new Object[ numberOfNeighbors ]; What does this mean?... Flag and hold off... // use a random number to decide which one to interact with ArrayList neighborNumbers = new ArrayList(); initPositions(); populateSiteGrid(); initSiteListeners(); What does this mean?... Flag and hold off...// we have liftoff bResume = true; bRunning = true; while ( bRunning ) { if ( bResume ) { moveEntities(); What does this mean?... Flag and hold off...// the resources regrow or dies out renewResources(); removeDepletedResources(); Comment and method name mismatch - rename method, remove comment//// have entites eat resourceFeeder(); cloneEntities(); What does this mean?... Flag and hold off...// reset for next round of interactions for ( Iterator e = entities(); e.hasNext(); ) ( (IEntity) e.next() ).reset(); for ( Iterator iter = entities(); iter.hasNext(); ){ IEntity entity = (IEntity) iter.next(); int xPos = entity.getX(); int yPos = entity.getY(); // check for all neighbors, but only interact with one neighbors[ 0 ] = siteGrid[ xPos + 1 ][ yPos + 0 ]; neighbors[ 1 ] = siteGrid[ xPos + -1 ][ yPos + 1 ]; neighbors[ 2 ] = siteGrid[ xPos + 0 ][ yPos + 1 ]; neighbors[ 3 ] = siteGrid[ xPos + 1 ][ yPos + 1 ]; // neighbors[ 4 ] = siteGrid[ xPos + -1 ][ yPos + -1 ]; // neighbors[ 5 ] = siteGrid[ xPos + 0 ][ yPos + -1 ]; // neighbors[ 6 ] = siteGrid[ xPos + 1 ][ yPos + -1 ]; // neighbors[ 7 ] = siteGrid[ xPos + -1 ][ yPos + 0 ]; What does this mean?... Flag and hold off... //get the entities right, bottom-left bottom, and bottom-right neighbor // for( int i = 0; i < 3; i++ ){ // neighbors[ i ] = siteGrid[ xPos + ((i + 1) / 2)][ yPos + ((i+1) % 2 )]; // }// for loop What does this mean?... Flag and hold off...// used for picking a random neighbor to interact with for ( int i = 0; i < numberOfNeighbors; i++ ) neighborNumbers.add( new Integer( i ).toString() ); while( neighborNumbers.size() > 0 ) { Comment does not correspond to the code... Flag, revisit //if there was something next to the right, bottom, bottom-left //of bottom-right of the entity int i = Integer.parseInt( (String) neighborNumbers.remove( new Double( Math.random() * neighborNumbers.size() ).intValue() ) ); if( neighbors[ i ] != null ){ if( neighbors[ i ] instanceof IEntity ) { IEntity neighbor = (IEntity) neighbors[ i ]; if ( entity.isAlive() && neighbor.isAlive() && !neighbor.isInteracted() ) { interact( entity, neighbor); } } } // first if } } removeOldDeadEntitiesRemains(); checkMortality(); removeDeadEntities(); addChildEntities(); incrementAge(); if ( siteRenderer != null ) { siteRenderer.SiteChanged(); statisticsManager.SiteChanged(); } // slow down simulation if the user is viewing it try { if ( siteRenderer != null && siteRenderer.isSiteViewable() ) { Thread.sleep( DELAY_CONST ); } } catch( Exception e ) { Log.error( "Error in sleeping" ); } } else { try { Thread.sleep( QUERY_CONST ); } catch( Exception e ) { Log.error( "Error in sleeping" ); } } } } Okay, now first super easy refactoring. Rename the method so that its name clearly describes what it does. Important thing to remember is that refactoring is all about simple easy to manage and to understand methods. Smaller well named methods reduce need for comments. Comment and method name mismatch - rename method, remove comment//// have entites eat resourceFeeder(); becomes (note that the comment is gone!) makeEntitiesEat(); Next step, find unused vars. The easiest way to confirm unused variable is to comment it out and recompile. In this code all things are used. Next, find vars declared far away from the places where they are used. To do this simply search for occurences. We find that: int numberOfNeighbors = 4; Object neighbors[] = new Object[ numberOfNeighbors ]; ArrayList neighborNumbers = new ArrayList() Are used several light years away inside of some for loops. Lets move them closer, to the beginnnings of these loop. Side note: Please for your own sake, do not worry about the performance. Now the code looks like this: public void run() { initPositions(); populateSiteGrid(); initSiteListeners(); What does this mean?... Flag and hold off...// we have liftoff bResume = true; bRunning = true; while ( bRunning ) { if ( bResume ) { moveEntities(); What does this mean?... Flag and hold off...// the resources regrow or dies out renewResources(); removeDepletedResources(); Comment and method name mismatch - rename method, remove comment//// have entites eat resourceFeeder(); cloneEntities(); What does this mean?... Flag and hold off...// reset for next round of interactions for ( Iterator e = entities(); e.hasNext(); ) ( (IEntity) e.next() ).reset(); int numberOfNeighbors = 4; Object neighbors[] = new Object[ numberOfNeighbors ]; for ( Iterator iter = entities(); iter.hasNext(); ){ IEntity entity = (IEntity) iter.next(); int xPos = entity.getX(); int yPos = entity.getY(); // check for all neighbors, but only interact with one neighbors[ 0 ] = siteGrid[ xPos + 1 ][ yPos + 0 ]; neighbors[ 1 ] = siteGrid[ xPos + -1 ][ yPos + 1 ]; neighbors[ 2 ] = siteGrid[ xPos + 0 ][ yPos + 1 ]; neighbors[ 3 ] = siteGrid[ xPos + 1 ][ yPos + 1 ]; What does this mean?... Flag and hold off...// used for picking a random neighbor to interact with What does this mean?... Flag and hold off... // use a random number to decide which one to interact with ArrayList neighborNumbers = new ArrayList(); for ( int i = 0; i < numberOfNeighbors; i++ ) neighborNumbers.add( new Integer( i ).toString() ); while( neighborNumbers.size() > 0 ) { Comment does not correspond to the code... Flag, revisit //if there was something next to the right, bottom, bottom-left //of bottom-right of the entity int i = Integer.parseInt( (String) neighborNumbers.remove( new Double( Math.random() * neighborNumbers.size() ).intValue() ) ); if( neighbors[ i ] != null ){ if( neighbors[ i ] instanceof IEntity ) { IEntity neighbor = (IEntity) neighbors[ i ]; if ( entity.isAlive() && neighbor.isAlive() && !neighbor.isInteracted() ) { interact( entity, neighbor); } } } // first if } } removeOldDeadEntitiesRemains(); checkMortality(); removeDeadEntities(); addChildEntities(); incrementAge(); if ( siteRenderer != null ) { siteRenderer.SiteChanged(); statisticsManager.SiteChanged(); } // slow down simulation if the user is viewing it try { if ( siteRenderer != null && siteRenderer.isSiteViewable() ) { Thread.sleep( DELAY_CONST ); } } catch( Exception e ) { Log.error( "Error in sleeping" ); } } else { try { Thread.sleep( QUERY_CONST ); } catch( Exception e ) { Log.error( "Error in sleeping" ); } } } } Now, the next things is to turn lots of long loops into separate methods. For example: for ( Iterator e = entities(); e.hasNext(); ) ( (IEntity) e.next() ).reset(); while looks weird and I don't understand why and what the heck, but I can get it out of the way, by using Extract Method technique. resetEntities(); ... private void resetEntities() { for ( Iterator e = entities(); e.hasNext(); ) { ( (IEntity) e.next() ).reset(); } } Now I am kind of stuck.... The thing is that I definitily feel that I can do much more, but these nested while loops look daunting... Not to worry, lets take them out from inside one at a time. Lets look at the inner most loop: while( neighborNumbers.size() > 0 ) { //if there was something next to the right, bottom, bottom-left //of bottom-right of the entity This looks really suspicious, conversions are alarming... int i = Integer.parseInt( (String) neighborNumbers.remove( This looks like generate next random number in range new Double( Math.random() * neighborNumbers.size() ).intValue() ) ); if( neighbors[ i ] != null ){ if( neighbors[ i ] instanceof IEntity ) { IEntity neighbor = (IEntity) neighbors[ i ]; This looks like condition for interaction if ( entity.isAlive() && neighbor.isAlive() && !neighbor.isInteracted() ) { interact( entity, neighbor); } } } } We are back in business, we just spotted 3 weid things. Lets deal with them one at time. This looks like generate next random number in range new Double( Math.random() * neighborNumbers.size() ).intValue() ) ); This really belongs to some MathUtils class. We need to extract method and move it into another class. But wait, look closer, all we want is the next random integer in range! This is already part of the Random API (know thy API!). All we have to do is to create a static Random on the bottom of this class and we are good to go. Before: This looks like generate next random number in range new Double( Math.random() * neighborNumbers.size() ).intValue() ) ); After: rnd.nextInt( neighborNumbers.size() ); Done, now next another technique replacing complex content of if statement with a method: Before: This looks like condition for interaction if ( entity.isAlive() && neighbor.isAlive() && !neighbor.isInteracted() ) { After: if ( canInteract( entity, neighboor ) ) { ... } ... private boolean canInteract( IEntity entity, IEntity neighboor ) { return ( entity.isAlive() && neighbor.isAlive() && !neighbor.isInteracted() ); } Now that loop looks better, and we still have work to do: while( neighborNumbers.size() > 0 ) { //if there was something next to the right, bottom, bottom-left //of bottom-right of the entity This looks really suspicious, conversions are alarming... int i = Integer.parseInt( (String) neighborNumbers.remove( rnd.nextInt( neighborNumbers.size() ); Hmm... we no long need this if if( neighbors[ i ] != null ){ if( neighbors[ i ] instanceof IEntity ) { IEntity neighbor = (IEntity) neighbors[ i ]; This looks like condition for interaction if ( canInteract( entity, neighboor ) ) { interact( entity, neighbor); } } } } Well here is something interesting, we no longer need one of the if statements! Can't be instanceof of IEntity if it is null - out it goes: while( neighborNumbers.size() > 0 ) { //if there was something next to the right, bottom, bottom-left //of bottom-right of the entity This looks really suspicious, conversions are alarming... int i = Integer.parseInt( (String) neighborNumbers.remove( rnd.nextInt( neighborNumbers.size() ); if( neighbors[ i ] instanceof IEntity ) { IEntity neighbor = (IEntity) neighbors[ i ]; This looks like condition for interaction if ( canInteract( entity, neighboor ) ) { interact( entity, neighbor); } } } Why are we parsing here? What was the declaration? ArrayList neighborNumbers = new ArrayList(); for ( int i = 0; i < numberOfNeighbors; i++ ) neighborNumbers.add( new Integer( i ).toString() ); Okay, let me get this straight. We are creating integers converting them to strings, which we then convert back to integers?! So 5 + 10 = 15 - 10 = 5. I see. Get rid of this conversion all together: ArrayList neighborNumbers = new ArrayList(); for ( int i = 0; i < numberOfNeighbors; i++ ) { neighborNumbers.add( new Integer( i )); } Now the bottom loop looks like this: while( neighborNumbers.size() > 0 ) { //if there was something next to the right, bottom, bottom-left //of bottom-right of the entity Still looks weird, why do I need objects? int i = ((Integer)neighborNumbers.remove( rnd.nextInt( neighborNumbers.size() ).intValue(); if( neighbors[ i ] instanceof IEntity ) { IEntity neighbor = (IEntity) neighbors[ i ]; This looks like condition for interaction if ( canInteract( entity, neighboor ) ) { interact( entity, neighbor); } } } Of course we do not need objects, we just need array of ints, here it comes: int[] neighborNumbers = new int[ numberOfNeighbors ]; for ( int i = 0; i < numberOfNeighbors; i++ ) { neighborNumbers[ i ] = i; } Now the bottom loop looks like this: while( neighborNumbers.size() > 0 ) { //if there was something next to the right, bottom, bottom-left //of bottom-right of the entity int i = neighborNumbers[ rnd.nextInt( neighborNumbers.size() ) ]; if( neighbors[ i ] instanceof IEntity ) { IEntity neighbor = (IEntity) neighbors[ i ]; if ( canInteract( entity, neighboor ) ) { interact( entity, neighbor); } } } Okay now, a really philosophical question... Why do I need this array, what am I trying to do? All I need is a number between 0 and numberOfNeighbors, right!? Scarry as it sounds, the array goes and now the bottom loop becomes: while( neighborNumbers.size() > 0 ) { //if there was something next to the right, bottom, bottom-left //of bottom-right of the entity int i = rnd.nextInt( neighborNumbers.size() ); if( neighbors[ i ] instanceof IEntity ) { IEntity neighbor = (IEntity) neighbors[ i ]; if ( canInteract( entity, neighboor ) ) { interact( entity, neighbor); } } } Hold on, we broke something... The logic there said that we have to do it 4 times, for each neighboor. Now this is not supported by the comment: //if there was something next to the right, bottom, bottom-left //of bottom-right of the entity What do we do? Well it seems to me that it makes sense for the entity to interact with each neighboor. So we do not need random at all. Taking it out we get: for ( int i = 0; i < numberOfNeighbors; i++ ) { if( neighbors[ i ] instanceof IEntity ) { IEntity neighbor = (IEntity) neighbors[ i ]; if ( canInteract( entity, neighboor ) ) { interact( entity, neighbor); } } } Not bad. Lets make it into a method: private void interactWithNeighboors( IEntity entity, IEntity[] neighboors ) { for ( int i = 0; i < numberOfNeighbors; i++ ) { if( neighbors[ i ] instanceof IEntity ) { IEntity neighbor = (IEntity) neighbors[ i ]; if ( canInteract( entity, neighboor ) ) { interact( entity, neighbor); } } } Now the larger loop look like this: for ( Iterator iter = entities(); iter.hasNext(); ){ IEntity entity = (IEntity) iter.next(); int xPos = entity.getX(); int yPos = entity.getY(); // check for all neighbors, but only interact with one neighbors[ 0 ] = siteGrid[ xPos + 1 ][ yPos + 0 ]; neighbors[ 1 ] = siteGrid[ xPos + -1 ][ yPos + 1 ]; neighbors[ 2 ] = siteGrid[ xPos + 0 ][ yPos + 1 ]; neighbors[ 3 ] = siteGrid[ xPos + 1 ][ yPos + 1 ]; interactWithNeighboors( entity, neighboors ); } Well now, this neighbors array is quite useless, is it not?! We can now make things cleaner like this: for ( Iterator iter = entities(); iter.hasNext(); ){ IEntity entity = (IEntity) iter.next(); int xPos = entity.getX(); int yPos = entity.getY(); interactWithNeighboor( entity, xPos + 1, yPos ); interactWithNeighboor( entity, xPos - 1, yPos ); interactWithNeighboor( entity, xPos, yPos + 1 ); interactWithNeighboor( entity, xPos, yPos - 1 ); } ... private void interactWithNeighboors( IEntity entity, int x, int y) { Object oNeighboor = siteGrid[ x, y ]; if( oNeighboor instanceof IEntity ) { IEntity neighbor = (IEntity) neighbors[ i ]; if ( canInteract( entity, neighboor ) ) { interact( entity, neighbor); } } } Of course this would not be complete without one more ExtractMethod: handleNeighboorInteractions(); ... private void handleNeighboorInteractions() { for ( Iterator iter = entities(); iter.hasNext(); ){ IEntity entity = (IEntity) iter.next(); int xPos = entity.getX(); int yPos = entity.getY(); interactWithNeighboor( entity, xPos + 1, yPos ); interactWithNeighboor( entity, xPos - 1, yPos ); interactWithNeighboor( entity, xPos, yPos + 1 ); interactWithNeighboor( entity, xPos, yPos - 1 ); } And the method now looks like this: public void run() { initPositions(); populateSiteGrid(); initSiteListeners(); // we have liftoff bResume = true; bRunning = true; while ( bRunning ) { if ( bResume ) { moveEntities(); renewResources(); removeDepletedResources(); makeEntitiesEat(); cloneEntities(); resetEntities(); handleNeighboorInteractions(); removeOldDeadEntitiesRemains(); checkMortality(); removeDeadEntities(); addChildEntities(); incrementAge(); if ( siteRenderer != null ) { siteRenderer.SiteChanged(); statisticsManager.SiteChanged(); } try { if ( siteRenderer != null && siteRenderer.isSiteViewable() ) { Thread.sleep( DELAY_CONST ); } } catch( Exception e ) { Log.error( "Error in sleeping" ); } } else { try { Thread.sleep( QUERY_CONST ); } catch( Exception e ) { Log.error( "Error in sleeping" ); } } } Okay, we are going good. There is more to be done, but you get the feel for it. Important things for you to remember is to recompile and test often. Do not make large changes, you will probably break things. Final note on reactoring. As a personal favor to me: Please DO NOT right code which is ugly and convoluted.