Get the width and height of a shape as user input

Question

I made this program to understand inheritance better.

Now that I'm at this basic level it's not really a problem but I can see this coming in the future and so I'm asking what's the best method to ask the user to insert the width and height of a rectangle (in this case).

The code I'm providing includes the simple way, by simply asking the user and inserting the number in a variable, then passing the variable to the setter inside the object (rect1).

I think when I will add up more shapes this can become a little confusing, is there a better way I can ask the user to input the number?

Since I'm asked to provide more context: I'm generally asking what's the best or a better method to ask input from the user that is going inside an object.

This is my version of it, that I think is good in this simple level but it will get messy with more shapes or generally more classes, I use 2 variable to ask for 2 dimensions, I will need 1 variable for every dimension and when I will create an object if I pass the wrong variable the program will work anyway, giving the wrong result.

Another way that I thought is insert an "askSomething" method in each class so that I don't need variables and every time a new object is created the io stream will be asked automatically. The "AskSomething" method is identical to a setter except it has cout and cin already inside

I don't really came up with other ideas.

The code I'm providing is a simple program that by asking height and width of a Rectangle calculate its Area, but of course by the class name Rectangle, the member names height and width and the method name getArea was pretty obvious.

main.cpp:

int main() {
 Rectangle rect1;
 int width, height;
 std::cout << "Insert the width of the rectangle: ";
 std::cin >> width;
 std::cout << "Insert the height of the rectangle: ";
 std::cin >>height;
 rect1.setHeight(height);
 rect1.setWidth(width);
 std::cout << "The area of the rectangle is: " << rect1.getArea() << std::endl;
 std::cout << "Drawing: " << std::endl;
 std::cout << " ";
 for (int i = 0; i < width+1; ++i) {
 std::cout << "-";
 }
 std::cout << std::endl;
 for (int j = 0; j < (height+1)/2; ++j) { // divided by 2 to have a better proportion
 std::cout << "|";
 for (int i = 0; i < width+1; ++i) {
 std::cout << " ";
 }
 std::cout << "|";
 std::cout << std::endl;
 }
 std::cout << " ";
 for (int i = 0; i < width+1; ++i) {
 std::cout << "-";
 }
 return 0;
}

Shape.h:

class Shape {
public:
 int getWidth() const {
 return width;
 }
 void setWidth(int width) {
 Shape::width = width;
 }
 int getHeight() const {
 return height;
 }
 void setHeight(int height) {
 Shape::height = height;
 }
protected:
 int width, height;
};

Rectangle.h:

class Rectangle: public Shape {
public:
 int getArea(){
 return width*height;
 }
};

Thanks to everyone! :)

• pacmaninbw: I'm not the person with the -1 but my guess would be that it is because you are missing the point of the code review site, we review working code and provide suggestions on how to improve the code. The problems with this question:1)we won't tell you how to do something new because that means the code isn't working as expected, 2) What's the best way leads to opinion based answers. Now your own suggestions to yourself are on the correct path. Please read codereview.stackexchange.com/help/how-to-ask
• AlexanderWalls: I'm asking the best way to ask a user an input that is gonna be inserted in a member of an object... That's a general question and I don't even know the options. I'm asking the best way of doing this, I'm not asking the best way between some options. If I can't learn new thing or asking something what's the point of the forum?
• pacmaninbw: Did you read the guidelines for the code review site at codereview.stackexchange.com/help/how-to-ask and the other help pages at codereview.stackexchange.com/help? If you want how to answers this is the wrong site, you should try stackoverflow.com.
• AlexanderWalls: Already tried, they said to come here
• L. F.: Welcome to Code Review! This question is incomplete. To help reviewers give you better answers, please add sufficient context to your question. The more you tell us about what your code does and what the purpose of doing that is, the easier it will be for reviewers to help you. Questions should include a description of what the code does.

Answer 1

Here are a number of things that may help you improve your code.

Provide complete code to reviewers

This is not so much a change to the code as a change in how you present it to other people. Without the full context of the code and an example of how to use it, it takes more effort for other people to understand your code. This affects not only code reviews, but also maintenance of the code in the future, by you or by others. In this case, for example, there are no #include directives anywhere.

Use the appropriate #includes

In order to compile and link, the main code requires the following two lines:

#include "Rectangle.h"
#incldue <iostream>

It was not difficult to infer, but see the point above.

Don't write getters and setters for every class

C++ isn't Java and writing getter and setter functions for every C++ class is not good style. Instead, move setter functionality into constructors and think very carefully about whether a getter is needed at all. In this code, getters for Shape are ever used, which emphasizes why they probably shouldn't be written in the first place. See C.131 for more details.

Rethink the class design

A class is useful because it allows us to encapsulate data and functions and assure that any necessary constrains (called invariants in computer science) are always enforced. For example, we might specify a shape for which the area must always be at least 7. However this shape is not that, so there's not much point in making the width and height as protected or private here. It makes even less sense in this particular case because the setters and getters mentioned above mean that any other piece of code can arbitrarily modify those values anyway.

Don't use std::endl if you don't really need it

The difference betweeen std::endl and '\n' is that '\n' just emits a newline character, while std::endl actually flushes the stream. This can be time-consuming in a program with a lot of I/O and is rarely actually needed. It's best to only use std::endl when you have some good reason to flush the stream and it's not very often needed for simple programs such as this one. Avoiding the habit of using std::endl when '\n' will do will pay dividends in the future as you write more complex programs with more I/O and where performance needs to be maximized. See Sl.io.50 for more details.

Think carefully about signed vs. unsigned integers

What would it mean if a rectangle had a negative number for its width or height? If you don't have a good answer for that question, it might be better to have those data members be unsigned instead of int.

Separate input, output and calculation

To answer your main question, to the degree practical it's usually good practice to separate input, output and calculation for programs like this. By putting them in separate functions, it isolates the particular I/O for your platform (which is likely to be unique to that platform or operating system) from the logic of the objects (which does not depend on the underlying OS). So having both the input and display of the rectangle in main rather than incorporated into the Rectangle object is probably generally the better choice. However, see the next point.

Decompose your program into functions

All of the logic here is in main except for the relatively trivial calculation of area. I'd suggest instead writing functions so that your main could look like this:

int main() {
 int width = get_prompted_int("Insert the width of the rectangle: ");
 int height = get_prompted_int("Insert the height of the rectangle: ");
 Rectangle rect1(width, height);
 std::cout << "The area of the rectangle is: " 
 << rect1.getArea() 
 << "\nDrawing:\n";
 draw(std::cout, rect1);
}

• AlexanderWalls: First, thanks for the complete and incredibly useful answer that I searched for the entire morning. Second, I have a few questions to ask if you would like to answer.
• Edward: Sure, if there are further questions that pertain to this code ask here and I'll try to clarify my answer. If they're about some other code, you can post a new question. Try to make sure that any new question meets all of the site guidelines. See How to Ask.
• AlexanderWalls: Sure, principally related to what you said: 1) "while std::endl actually flushes the stream", I'm not English nor American and I don't really understood the meaning of this phrase. I only understood that \n is more likely to be used. 2) "Don't write getters and setters for every class". Our professor told us to always use Getters and Setters in order to provide a better security of the code. I can see them inside the c'tor in this case, but can't really understand why I shouldn't use Getters/Setters and when.
• Edward: I've added some references to the relevant portions of the C++ Core Guidelines to clarify.
• AlexanderWalls: Thanks a lot, also seems a great reading, pretty fundamental :)

Answer 2

I think you have chosen really bad way to model the inheritance relation between a rectangle and a shape.

I believe that every shape has an area. Unlike your Shape class, which does not allow this, only the Rectangle class does.

On other hand, I believe that not all shapes have width and height. If anything, they have a smallest outer rectangle which has a width and height. Unlike, your Shape class which always has width and height and allows to set it to any arbitrary value.

You should turn it inside out.

You can still have the shape be able to give you the smallest outer rectangle (with its width and height), but implementing that method for anything except the Rectangle (where it would just return copy of itself) would be a bit more complex and providing such code for you would go far beyond a code review :) But that way you can provide the width/height pair in a read only manner, making sure that it cannot be modified for shapes where it would lead to corruption (like setting width!=height for a circle).

I would also rather use double for the numeric values as you would soon find out that areas of shapes are not always integers even for shapes which themselves are defined with integers (although this is not the case for rectangles).

class Rectangle;
class Shape
{
public:
 virtual ~Shape() {}
 virtual double getArea() const = 0;
 virtual Rectangle getSmallestOuterRectangle() const = 0;
};
class Rectangle : public Shape
{
public:
 double width, height;
 Rectangle(double width, double height) : width(width), height(height) {}
 virtual ~Rectangle() {}
 virtual double getArea() const {return width * height;}
 virtual Rectangle getSmallestOuterRectangle() const {return *this;}
};

Further obviously, you cannot create all shapes from just height and width. If I wanted to create a circle, two variables are too much. If I wanted to create an arbitrary triangle, two variables are not enough.

In any way, having the Shape/Rectangle/Circle/Triangle classes know anything about streams is a mistake. You can always read the values into variables and pass them through constructor or update the respective property (if you made them public, which is ok here I believe, no setters/getters are needed).