Calculating rectangular area

Question

This program is working correctly, but I need to check if the structure is good or can be improved. Plus, if I called ra.area() immediately after declaring ra, I get 687194768. Why is that?

#include "stdafx.h"
#include "iostream"
using namespace std;
class rectangleclass
{
 int w,h;
public:
void setnum(int x, int y)
{
w=x;
h=y;
}
int area() 
{
 return w*h;
}
};
int main()
{
rectangleclass ra;
int l;
int L;
cout<<"please write the height and width in m\n" ;
cin>>L;
cin>>l;
ra.setnum(L,l);
cout<<ra.area()<<"m^2";
 system("pause");
 }

• Jamal: why is that? - so something is not quite working as expected?
• holroy: @Jamal, The code is working, but the OP is asking about an issue related to uninitialize variables when calling code out of order. Not a reason for closing the question!
• Loki Astari: @Rida What do you think the values of the member variables are after construction? Remember that in C++ unless you explicitly ask for something the compiler will not do it.

Answer 1

Readability

First things first. Formatting. Indent after each brace, consistently. It is more or less impossible to read your code.

Secondly, variable naming. 1 (el) is a terrible name for a variable, since it looks exactly like a l (one), so much so that I just flipped them and you probably didn't notice. Not to mention that one of your Ls is really width. So you should name them length and width.

Rectangle

Rather than having setnum(), which is not a meaningful name, instead require your Rectangle to be constructed directly with the values:

class Rectangle {
public:
 Rectangle(int w, int h)
 : width_(w), height_(h)
 { }
private:
 int width_, height_;
};

Note the improved naming of the member variables and the class. Adding class to the type name is unnecessary and redundant. This constructor will additionally solve your other problem: accidentally misusing Rectangle by accessing its member functions before you set any of the values (uninitialized ints have indeterminate value). This way, you're guaranteeing that width_ and height_ are set.

Usage here would be like:

cin >> width;
cin >> height;
Rectangle ra(width, height);
cout << ra.area();

const-ness

The area() member function doesn't (and shouldn't) modify any of its members. So it'd be better to mark it const:

int area() const { ... }

This lets you get the area of a const Rectangle.

• holroy: I'm damaged, I noticed your flip operation of 1 and l, straight away... :-)
• holroy: Just asking, is having the underscore after the variable normal in C++? I thought that was mainly used in Python (or other languages) to make a distinction between a reserverd word and a variable, like class_. I thought the normal convention was that private variable has the underscore in front of the variable, like _width?
• Rida Helbawi: sorry the L and l thing was because im a french educated , and they stand to longeur and largeur... and its a small program so i didnt give importance to variables...
• Rida Helbawi: everything is well understood and helpfull thank you, but i would like to ask about the underscore....is it a need or what
• Barry: The underscore is just a fairly common naming convention for private member variables. Some people put it up front, some people put it at the end. Some people don't put it anywhere. I have no strong feelings about it.

Answer 2

• Try not to use using namespace std.

• iostream is a system header, so it should be surrounded by angle brackets as such:

  #include <iostream>
  

  Quotes are only used for user headers.

• system("PAUSE") isn't very good to use for a number of reasons, mostly relating to performance and portability.

  Here's one alternative that avoids these problems:

  std::cin.get();
  

  It'll ask for a character input (with Enter) instead of any key by itself, however.

Answer 3

Some more notes related to naming variables:

You should choose a naming standard, and stick to it. Whichever you stick to, try to be as consistent as possible.

In C# there is one naming standards stating that private variables starts with an underscore, and when Barry suggested ending with it I wondered why a trailing underscoring instead of a leading underscore. After some research I found amongst many answers this one.

Quoting only a bit from the top:

The rules (which did not change in C++11):

• Reserved in any scope, including for use as implementation macros:
  • identifiers beginning with an underscore followed immediately by an uppercase letter
  • identifiers containing adjacent underscores (or "double underscore")
• Reserved in the global namespace:
  • identifiers beginning with an underscore

So it seems like in C++ you are not supposed to start off with an underscore, but rather end with them, if you choose to use them. This is further elaborated in some of the other answers given there.

Is it important to use trailing underscores for private variables, or not?
Well, it can help readability and clarity in your program. Here is a small code example:

void setSize(int width, int height)
{
 width_ = width;
 this->height = height; 
}

Due to naming conventions the first line is (somewhat) clear that it sets the private instance variable to method parameter. The second case is a little more unclear (when not used to syntax), but it does do the job of setting the instance variable height to the parameter height (due to the usage of this->).

But the most important aspect is to try to avoid single-letter variable names, with an exception for loop variables in some cases. Well-named variables helps reading and understanding your code.