2

I am currenty doing a Sudoku game in c++. For that , I defined many classes including, class Pos for position, and class Error for maintaining errors.

Now, in class Pos,

class Pos {
    int x;
    int y;
public:
    Pos();
    Pos(Pos const&);
    Pos(int,int);

    void    setPos(int,int);

    void    setx(int);
    void    sety(int);

    int    getx() const ;
    int    gety() const ;

    string getPosReport();

    virtual ~Pos();

}; 

What I have done: I have restricted the value for x and y to lie between 0-9 by writing setx and sety as,

void Pos::setx(int x){
    if((x < 0) & (x > 9))
        throw  Exception("Invalid Position");
    else
        this->x = x;
}

void Pos::sety(int y){
    if((y < 0) & (y > 9)){
        throw Exception("Invalid Position");
    }
    else
        this->y = y;
}

My Problem: How Pos() must be defined.

What I have worked:

  1. Removing default constructor, didnt helped me because,

    class Error {
        string errmsg;    
        Pos pos1,pos2; // Pos() must be called!! default constructor cannot be removed!
        bool isError;
        // ...... 
    }
    
  2. I can initialize x and y to -1 in Pos() , but I feel that will bring instability in my code.

  3. I can have int* x and int* y in class Pos such that I can have x = y = NULL, but I feel Pointers will make my code relatively complex than now.

    so,it will be better if some one gives a solution which stimulates the scenario like x = y = N/A (not applicable)

Lightness Races in Orbit
  • 378,754
  • 76
  • 643
  • 1,055
Muthu Ganapathy Nathan
  • 3,199
  • 16
  • 47
  • 77
  • 3
    This question is quite wide-ranging. I suggest you get your hands on a [good C++ book](http://stackoverflow.com/questions/388242/the-definitive-c-book-guide-and-list). – Etienne de Martel Feb 02 '12 at 17:08
  • 1
    Any particular reason you cannot use `Pos pos1(0,0),pos2(1,4)`? – Vyktor Feb 02 '12 at 17:10
  • Also, I see you got bored by the end of writing your post; what a mess! – Lightness Races in Orbit Feb 02 '12 at 17:14
  • Sudoku must start at some position, why don't you just initialize with that? – Sid Feb 02 '12 at 17:21
  • For some little things, use std::string not string in a header file as you are not "using namespace std". Don't make destructors virtual for no reason, and there is no reason to make this one virtual. getPosReport() should almost certainly be const. Copy constructor and destructor do not need overload. – CashCow Feb 02 '12 at 17:35
  • 1
    @Vyktor @Sid It is a team project. Hence as **Best is always better that Good** what I feel is, if I defaulty initialize to 0,0 or 1,4 , my team mates will felt uneasy when they use `Pos()` bcz they may not remember the default values all the time. – Muthu Ganapathy Nathan Feb 02 '12 at 18:04

4 Answers4

4

Give Error only constructors that initialise the Pos member objects. Then you won't need to default-construct Pos anywhere.

Initialising members is done with the "member initialisation list", which you may find after the colon (:) below.

class Error {
   Pos p;

   public:
     Error() : p(1,2) {};
};

This should be covered in your book.


  • Top credit for correctly identifying valid solutions, and the reasons not to adopt them.
Lightness Races in Orbit
  • 378,754
  • 76
  • 643
  • 1,055
  • @EAGER_STUDENT: Why only partially? – Lightness Races in Orbit Feb 02 '12 at 18:31
  • Because, `Error::Error() : pos1(0,0) ,pos2(1,2) { errmsg = "No error"; isError = false; }` says like, position `0,0` and `1,2` has error. Thats why. Now I have a doubt whether one can set `0,0` `1,2` as default values eventhough no position is mentioned in `Error()`. whether it will be a good design? – Muthu Ganapathy Nathan Feb 02 '12 at 18:57
  • @EAGER_STUDENT: Oh, good point. I forgot about that part of your question. I'd go with `boost::optional` then... or just not care. It doesn't have to be an "invalid" position; just an "initial" one. – Lightness Races in Orbit Feb 02 '12 at 19:06
2

You could use boost::optional in your Pos class too. That is a general way of doing "nulls" in C++.

I'm not saying it's really the best option here. Unless you really want an "uninitialized" state I would just use {0,0} as the "default" position.

CashCow
  • 30,981
  • 5
  • 61
  • 92
0

You need to either remove the default constructor and force users of your class to use the

Pos(int, int)

version of the constructor, or you need to initialise x and y to something like -1 and ensure you check there values when you use them.

The question to ask is, does it make sense to instantiate an invalid position? If not then remove the default constructor and force your object to be valid upon construction.

mcnicholls
  • 846
  • 5
  • 10
0

Well, for starters, when I implemented Sudoku, I just used an int (in the range [0, 81)) for the position. But...

This is a class with value semantics, so you probably do want some sort of default constructor. There are two valid approaches to this:

  • You can just choose some arbitrary position (e.g. 0, 0), and use this. In this case, you can simply provide appropriate default values for Pos(int, int), and let it serve as the default constructor as well. Or:

  • You can use sentinal values (-1 would be a good choice here, but any out of range value will do). In this case, you check for the sentinal values for any read access (except copy), and abort if someone tries to use an incorrectly initialized instance.

(And you want to abort in case of error, not throw an exception.)

James Kanze
  • 150,581
  • 18
  • 184
  • 329
  • I would say you want to assert, and this would happen only in debug mode. – CashCow Feb 02 '12 at 17:32
  • I would say you want assert as well. And assert is active any time you include `` without `NDEBUG` being defined. In most of the applications I've worked on, this has been always. Even in released code. – James Kanze Feb 02 '12 at 17:59
  • @James: [Orly?](http://sourceware.org/git/?p=glibc.git;a=blob;f=assert/assert.h;h=315487117f296fae89fb0c0eb7c4b38a4e8ac541;hb=HEAD) – Lightness Races in Orbit Feb 02 '12 at 18:33