HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppMinor

Mars rover movement and position

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
positionmovementroverandmars

Problem

I have written a solution of a Mars Rover problem in C++ using OOP, but I am not fully satisfied with the design.
What changes can I make to improve the design?

Problem description is:


A rover’s position and location is represented by a combination of x and y co-ordinates and a letter representing one of the four cardinal compass points. The plateau is divided up into a grid to simplify navigation. An example position might be 0, 0, N, which means the rover is in the bottom left corner and facing North.


In order to control a rover , NASA sends a simple string of letters. The possible letters are ‘L’, ‘R’ and ‘M’. ‘L’ and ‘R’ makes the rover spin 90 degrees left or right respectively, without moving from its current spot. ‘M’ means move forward one grid point, and maintain the same heading.

#include
#include
#include
using namespace std;

class Rover{

int x;
int y;
char orientation ;

public :

Rover();
Rover( int , int , char );
void rotateLeft();
void rotateRight();
void movePosition();
void displayPosition();

};    

Rover :: Rover()
{
x =0;
y =0;
orientation = 'N';

}

Rover :: Rover ( int positionX , int positionY , char Orientation )
{
x= positionX;
y= positionY;

orientation = Orientation ;
}

void Rover::  displayPosition()
{
cout>x>>y>>orient;
Rover firstRover(x,y,orient);

firstRover.displayPosition();

string roverMovement;
cin>>roverMovement;

for(int i=0 ; i < roverMovement.size() ; i++  )
{

     if(roverMovement[i]=='L')
     firstRover.rotateLeft();

     else if(roverMovement[i]=='R')
     firstRover.rotateRight();

     else
     firstRover.movePosition();
}

firstRover.displayPosition();

return 0;
}


How the design of this code can be improved?

Solution

The design is fine. You modeled the rover as a class, its actions with methods, and its state with member variables. This is natural, intuitive and simple, good this way.

On top of the issues Jerry already pointed out in his answer, I have a few additional comments about the implementation technique.

The rotation methods are a bit tedious, and tedious tasks are sometimes error prone. I would recommend a different approach for changing the orientation:

  • Create a simple string with the orientations in clockwise order, that is "NESW"



  • Forget about storing the name of the current orientation, it's enough to store its index in the orientations string. In all actions of the rover, we can work with this index and the string



  • When rotating right, the index of the target orientation is the index of the current +1 modulo length(orientations)



  • When rotating left, the index of the target orientation is the index of the current + length(orientations) -1 modulo length(orientations)



For moving forward, you could do similarly, by using arrays for the x and y movements depending on the index of the current orientation, so for x the content will be {0, 1, 0, -1}, and for y it will be {1, 0, -1, 0}.

The benefit of this approach will be faster actions, as there will be no more conditional statements (also called a table driven approach).

Context

StackExchange Code Review Q#133397, answer score: 8

Revisions (0)

No revisions yet.