patterncppMinor
Shuttle Puzzle solver
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
```
#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++?
Identifiers beginning with
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:
Why do you need a getter?
Methods should manipulate the object not expose the internals.
When you catch an exception you should catch by const reference:
Otherwise you put yourself in the position of slicing the object on the catch.
Did you really want to pass
Functions:
The functions:
Seem to be practically identical. You should be able to refactor into a single function.
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.