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

Shuttle Puzzle solver

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

Problem

How can I make this piece of code better? The problem to be solved is in the topmost comment.

```
#include
#include
#include
#include
#include
#include

/*
* Shuttle Puzzle:
* A Shuttle Puzzle of size 4 consists of 4 white marbles, 4 black marbles, and a strip of
* wood with 9 holes (as shown below). The marbles of the same color are placed in the
* holes at the opposite ends of the strip leaving the center hole empty. The object of the
* puzzle is to completely reverse the marbles on the strip. In general, a shuttle puzzle of
* size N consists of N white marbles, N black marbles and 2N+1 holes. There are only
* two permissible types of moves. You may slide 1 marble 1 space (into an empty hole)
* or jump 1 marble over 1 mar
*
* W W W W O B B B B
*/
//-------------------------------------------------------------------------------//
//Definition
class shuttle_puzzle
{
private :
/ Private member variables and typedefs /
typedef std::vector board;
board _game_board;
uint32_t _N;
enum SwapCase { LeftSkip = -2, ImmediateLeft = -1, Invalid = 0, ImmediateRight = 1, RightSkip = 2 };
bool _problem_solved;
std::queue _solutions;

/ Private Methods /
void has_left_swap_move(uint32_t open_loc, SwapCase* cases, board &check_board);
void has_right_swap_move(uint32_t open_loc, SwapCase* cases, board &check_board);
bool solved(uint32_t open_loc, board &check_board);
board swap_marbles(uint32_t open_location, SwapCase curr_case, board modify_board);

public :
/ ctor /
shuttle_puzzle(uint32_t N);

/ Method that solves the problem above /
void solve(uint32_t empty_location, board curr_game_board);

/ getter /
const board &get_game_board() const { return _game_board; }
uint32_t get_start_open_loc() const { return _N; }
const std::queue &get_solutions() const { return _solutions; }
}; //shuttle_puzzle

//-------------------------------------------------------------------------------//
//Implmentation

/*
* ctor
* Build

Solution

Small Syntax problems

Since all your other header files are C++, why not make this one C++?

#include 

// Change to 

#include 


Identifiers beginning with _ and then a capitol letter is reserved in all scopes.

uint32_t _N;


In general it is best to avoid underscore as the first letter of an identifier (as the rules are not obvious (unless you know where to look them up).

Passing pointers as parameters should be avoided:

void has_left_swap_move (uint32_t open_loc, SwapCase* cases, board &check_board);
void has_right_swap_move(uint32_t open_loc, SwapCase* cases, board &check_board);
                                  //        ^^^^^^^^^


Why do you need a getter?

const board &get_game_board() const { return _game_board; }
uint32_t get_start_open_loc() const { return _N; }
const std::queue &get_solutions() const { return _solutions; }


Methods should manipulate the object not expose the internals.

When you catch an exception you should catch by const reference:

catch (std::bad_alloc const& ba)


Otherwise you put yourself in the position of slicing the object on the catch.

Did you really want to pass curr_game_board by value?

void shuttle_puzzle::solve(uint32_t empty_location, board curr_game_board)


Functions:

The functions:

void shuttle_puzzle::has_left_swap_move(uint32_t open_loc, SwapCase *cases, board &check_board)
void shuttle_puzzle::has_right_swap_move(uint32_t open_loc, SwapCase *cases, board &check_board)


Seem to be practically identical. You should be able to refactor into a single function.

Code Snippets

#include <stdint.h>

// Change to 

#include <cstdint>
uint32_t _N;
void has_left_swap_move (uint32_t open_loc, SwapCase* cases, board &check_board);
void has_right_swap_move(uint32_t open_loc, SwapCase* cases, board &check_board);
                                  //        ^^^^^^^^^
const board &get_game_board() const { return _game_board; }
uint32_t get_start_open_loc() const { return _N; }
const std::queue<board> &get_solutions() const { return _solutions; }
catch (std::bad_alloc const& ba)

Context

StackExchange Code Review Q#11387, answer score: 5

Revisions (0)

No revisions yet.