C++ Why is this way dangerous?

Question

My peer tells me the way I implemented following is a dangerous practice. That I put same name for variable in main method and function. I mean as long as it works, isn't it ok? How would you have done differently?

Thanks.

#include <iostream>
#include <string>
#include <sstream>
#include "Three.h"

Three::Three(void)
{
}

Three::~Three(void)
{
}

void Three::rect (int& ar, int&vl, int len, int wid, int hgt)
{
    ar = (len * wid) * 2 + (len+wid) * 2 * hgt;
    vl = len * wid * hgt;
    cout << "Area is " << ar << " square feet that contains " << vl << " cubic feet." << endl;
}

char qt;

int main (int, char**) 
{
    int len = 0;
    int wid = 0;
    int hgt = 0;
    int ar = 0;
    int vl = 0;

    do
    {
        cout << "Length of House (ft): " << endl;
        std::cin >> len;
        cout << "Width of House (ft): " << endl;
        std::cin >> wid;
        cout << "Height of House (ft): " << endl;
        std::cin >> hgt;

        Three three;
        three.rect (ar, vl, len, wid, hgt);

        cout << "q, to quit" << endl; //My own quit statement
        std::cin >> qt;
    }
        while (qt != 'q'); 


}

Those two sets (main-local and Three::rect-parameters) do not overlap in name resolution space. This will be fine. — user7116, Feb 14 '13 at 20:13
I agree with Carl, nothing wrong with the code. I'd probably simply write `int main()`, and I find it odd that you have `char qt;` outside of the main function. — Prashant Kumar, Feb 14 '13 at 20:14
It is? http://stackoverflow.com/questions/1621574/mains-signature-in-c — Prashant Kumar, Feb 14 '13 at 20:16
@JanDvorak `int main()` is fine in C++. And it gets rid of some "unused variable" warnings. — juanchopanza, Feb 14 '13 at 20:16
@juanchopanza isn't that just an exception in the compiler that makes it work? — John Dvorak, Feb 14 '13 at 20:17
@JanDvorak No, it is one of two signatures allowed by the C++ standard. — juanchopanza, Feb 14 '13 at 20:19
> My peer tells me the way I implemented following is a dangerous practice. That I put same name for variable in main method and function. Has he said why? I can't fathom why anyone would think that. — Edward Strange, Feb 14 '13 at 20:12
it's not dangerous... it's bad code but not for the reason your peer told you. what the heck is *hgt*? and vl? after looking at the code, i can figure out what it is, but i shouldn't have to work that hard to get an overview... — thang, Feb 14 '13 at 21:05

score 7 · Accepted Answer · answered Feb 14 '13 at 20:12

as long as it works, isn't it ok? in general isn't a good mantra (especially in C/C++ where there's space for lots of seemingly working undefined behavior), but in this case I see nothing wrong.

There may be some potential for confusion if you have a local variable hiding a global and stuff like that, but here it's nothing like that - a parameter that happens to have the same name of the argument that is passed through it is not confusing and not "dangerous" at all.

Ah, confusion that is. Thank you. (Waiting for "accept an answer" opens up). — , Feb 14 '13 at 20:16

score 4 · Answer 2 · answered Feb 14 '13 at 20:11

4

There is absolutely nothing at all wrong with that practice. There are some minor weirdnesses with your code, but using the same variable names in two functions is not one of them.

answered Feb 14 '13 at 20:11

Carl Norum

219,201
40
422
469

I'll second the notion that there's nothing wrong with this practice. – David Hammen Feb 14 '13 at 20:13
4

actually, I would go one step farther and say that it helps the readability of the code in most (certainly not all) instances (this assumes that the variables are named in a manner than enhanced readability) – David Hope Feb 14 '13 at 20:13
2

@DavidHope: Exactly. This should happen all the time in your code if you name things for what they are. – GManNickG Feb 14 '13 at 20:17

score 1 · Answer 3 · answered Feb 14 '13 at 20:15

He probably meant that it is "dangerous" if you use the same variable names in a class function and in the class itself, because that will cause errors. This is not the case here though, and the code is fine.

score 1 · Answer 4 · answered Feb 14 '13 at 20:17

The more dangerous practice in your code is that you read from std::cin without ever checking whether any of the reads fail. If any do, you will be in an infinite waiting for qt to become 'q'.

Also, being able to use just cout suggests you might have a using namespace std; in Three.h which is not a recommended practice. (Perhaps it only has using std::cout; as you explicitly qualify std::cin?)

score 0 · Answer 5 · answered Feb 14 '13 at 20:16

0

Its Ok to do that. But remember to give better names to your variables. For example, rename wid to Width. normally i prefer to write input parameters of a function as in_Width. so that we know its an input for the current function. these things will matter when you start writing big projects. :) always try to have good practices.

It takes time at the time you are typing. But it saves you from lots of trouble when it comes to reviewing or debugging.. And on the other hand , it has no other significant cons.

answered Feb 14 '13 at 20:16

Deamonpog

805
1
10
24

2

Er, every argument to a function is always input. That should be the default, and it's a bit of a waste to write `in_` 99% of the time. For the rare occasion you use a reference parameter as an output (which you should avoid), the function name makes it clear enough that you don't need to mess with the variable names. – GManNickG Feb 14 '13 at 20:23

C++ Why is this way dangerous?

5 Answers5