   More intuitive Move( ) method?
  I store _row and _col internally in my "robot" object.I was wondering about this one piece of code and if there is a more intuitive or easier to understand way to code this piece:The idea is I want to move either North, South, East, or West, but I don't want to go to squares I have already been to.I want to try and make this sort of "English" like:if "Can go North" and "you have not been here before" then "go north, record last square"Notice that N, S, E and W  code is all very similar.  How can I make this more efficient?public void Move(){if((N != "robot") && (!this.HasBeenToSquare(new Square(_row - 1,_col)))){this.Row = _row - 1;this.Col = _col;}if((S != "robot") && (!this.HasBeenToSquare(new Square(_row + 1,_col)))){this.Row = _row + 1;this.Col = _col;}if((E != "robot") && (!this.HasBeenToSquare(new Square(_row,_col+1)))){this.Row = _row;this.Col = _col + 1;}if((W != "robot") && (!this.HasBeenToSquare(new Square(_row,_col-1)))){this.Row = _row;this.Col = _col - 1;}this._squares_visited.Add(new Square(this.Row,this.Col));_has_moved = true;}
  0) You do have a good instinct for wanting to improve on this. :)1) You seem to have both 'Row' and '_row' members that are supposed to be the same thing (similarly 'Col' and '_col'); that's asking for trouble.2) You seem to be using strings to represent the "type" of Squares and then comparing them with literals; that's also asking for trouble. Strings are an object type, so the ==, != comparisons etc. will use object identity - you want the .equals() method to be safe, but actually there are better representations of the type.2) Don't create the Square object in order to pass to HasBeenToSquare(), but instead let that function do the work. Again, when you compare the temporary Square to one within the visited list (I assume that's how the function works but perhaps I assume too much), you'd better be using a .equals() method - and you'd better have implemented that to do what you want.Probably a better idea is to store an array of boolean values, which represent whether or not a square has been visited.3) Caching these N,S,E,W values probably doesn't help much if any. In fact, it's hiding a bit of the repetition. What you're trying to do for each "candidate" location is:a) calculate a new position.b) Check if the Square at that location is ok to move to.c) If so, set our position to that calculated new position.Putting all of that together:// First, we make the helper function to detect if it's possible to move to a // particular location. There's a bit of a trade-off I'm making here - it's a // bit less understandable in the details, but cleaner overall - by having// this function change the current location and also return whether that// change occurred.private bool tryMoving(int newRow, int newCol) {  // First, do bounds checks.  if (newRow < 0 || newRow >= NUM_ROWS || newCol < 0 || newCol >= NUM_COLS) return false;  // Now get the Square at that location so that we can check if there's a  // Robot there.  Square target = getSquare(newRow, newCol); // not a newly created one, but  // one from the pre-existing map grid.  if (target.hasRobot()) return false; // the Square implements that by  // checking e.g. "return _type == ROBOT";  // Finally, see if we've been there before.  if (visitedArray[newRow * NUM_COLS + newCol]) return false;  // If we passed all the checks, we can move there.  visitedArray[newRow * NUM_COLS + newCol] = true;  _row = NUM_ROWS;  _col = NUM_COLS;  return true;}// Now movement is just a matter of trying to move in one direction, if it fails// trying the next direction, etc. And we can write this very compactly and// idiomatically:public void Move() {  _has_moved = tryMoving(_row - 1, _col) || tryMoving(_row + 1, _col) ||tryMoving(_row, _col - 1) || tryMoving(_row, _col - 1);  // As an added bonus, this correctly sets _has_moved to false if the robot is  // completely blocked in.};  First thing I see is that you are creating four temporary pointers to four Square objects - this might not be necessary.I don't know what your HasBeenToSquare function is doing with these pointers (comparing them to a list), but hopefully it is storing them or deleting them because otherwise the memory never gets freed.  You wouldn't want to do this anyway because the function is really just a boolean test and shouldn't be storing or deleting anything and it should probably be const function.  A good rule of thumb that I use (and I think is generally accepted is that you delete memory in the same scope in which you create it - unless you absolutely have to relinquish control of that memory for design reasons.Also are the N, S, E, W variables coming from?  Class members?  I don't think you can compare strings like this (unless they are string objects of some sort).  Maybe a boolean would be more appropriate and quiker? Or maybe even use a DIRECTION enumeration?Here's I would code this, given my limited knowledge of the interworkings of your class.enum DIRECTION { NONE = 0x0, NORTH, SOUTH, EAST, WEST };...public void Move(){// assuming _has_moved == falsetemp_square = new Square(_row, _col);    // Set to current squareswitch(d)// We are getting this d variable of DIRECTION type from somewhere in the class{case NORTH:--(temp_square->_row);if(!this.HasBeenToSquare(temp_square)){_has_moved = true;}break;// We're done :)case SOUTH:++(temp_square->_row);if(!this.HasBeenToSquare(temp_square)){_has_moved = true;}break;// We're done :)case EAST:++(temp_square->_col);if(!this.HasBeenToSquare(temp_square)){_has_moved = true;}break;// We're done :)case WEST:--(temp_square->_col);if(!this.HasBeenToSquare(temp_square)){_has_moved = true;}break;// We're done :)case NONE:// Fall through or do special processingdefault:break;}if(_has_moved){_row = temp_square->_row;_col = temp_square->_col;this._squares_visited.Add(temp_square);}// Assuming _squares visited is a vector or other std container,// Then I believe it creates a local copy of this object (even though you pass a pointer)// CORRECT ME IF I'M WRONG// So it is safe to delete this memorydelete temp_square;return;}Now I'm nowhere near an expert here but...In your version all of the comparisons are performed whether or not they need to.  For instance if we already established that we are moving North in the first comparison, we still do the remaining comparisons - this is a waste.  In my version if we are moving North we don't need to do the rest.  Also the comparisons are quicker as we are not comparing strings (which for you case 'robot' is actually AT LEAST 5 more comparisons - one for each letter).  If your algorithm produces one direction more than the others than you can optimize the switch statement to place those in order thus reducing the average comparisons per call.  Regardless you still average less comparisons per call with this method.In your version you are creating a new Square object 5 times (and potentially creating a memory leak), and doing 4 arithmetic operations (one for each object) no matter which direction we are moving.  In my version we create one Square object and do 1 - 4 arithmetic operations (depending on which direction we go).  We also clean up after ourselves.Again these are just knee jerk thoughts...I barely know what I'm talking about...   