returning const reference and compiler warning

[Dylan]

Just wondered what you guys do when you need a method that returns a
const reference to some data based upon some value.

something like this:

const CData& MyClass::GetDataWithValue(int iValue)const
{
for (int idx=0; idx<m_iNumRecords; ++idx)
{
if (m_Records[idx].GetValue() == iValue)
{
return m_Records[idx];
}
}
}

Most compilers will give a warning here. Something like "Warning: Not
all paths return a value"

This is fair enough but occasionally i know that this method will
*always* work correctly because there is no possible way iValue could
be anything else but valid. I'm certain this sort of thing crops up
for other programmers also. So my Q is: What do you do about it? (I
just ignore the warning at the moment but it irritates me!)

 

[velthuijsen]

Just wondered what you guys do when you need a method that returns a
const reference to some data based upon some value.

something like this:

const CData& MyClass::GetDataWithValue(int iValue)const
{
for (int idx=0; idx<m_iNumRecords; ++idx)
{
if (m_Records[idx].GetValue() == iValue)
{
return m_Records[idx];
}
}
}

Most compilers will give a warning here. Something like "Warning: Not
all paths return a value"

This is fair enough but occasionally i know that this method will
*always* work correctly because there is no possible way iValue could
be anything else but valid. I'm certain this sort of thing crops up
for other programmers also. So my Q is: What do you do about it? (I
just ignore the warning at the moment but it irritates me!)

I'd optimize the loop if you can guarantee that you always get a
result.
in this case:

int idx;
for (idx =0; m_records[idx].getValue != iValue; ++idx);
return m_records[idx];

Oh and that ; at the end of the for is not a mistake

 

[Victor Bazarov]

Just wondered what you guys do when you need a method that returns a
const reference to some data based upon some value.

something like this:

const CData& MyClass::GetDataWithValue(int iValue)const
{
for (int idx=0; idx<m_iNumRecords; ++idx)
{
if (m_Records[idx].GetValue() == iValue)
{
return m_Records[idx];
}
}

If you are certain that in a valid program the execution should never
come here, you need to (a) assert that:

assert( ! "Reached the point where value is invalid");

and (b) return a reference to, say, a static object. If everything goes
well, and your program never reaches this point, the object isn't even
initialised, and only its storage is zero-initialised:

static CData d;
return d;

}

Most compilers will give a warning here. Something like "Warning: Not
all paths return a value"

This is fair enough but occasionally i know that this method will
*always* work correctly because there is no possible way iValue could
be anything else but valid. I'm certain this sort of thing crops up
for other programmers also. So my Q is: What do you do about it?

See above.

> (I
just ignore the warning at the moment but it irritates me!)

And it should. If the code falls through, it has undefined behaviour if
the function that has non-void return type doesn't have a corresponding
'return' statement.

V

 

[Victor Bazarov]

[...]
I'd optimize the loop if you can guarantee that you always get a
result.
in this case:

int idx;
for (idx =0; m_records[idx].getValue != iValue; ++idx);

... m_records[idx].GetValue() != iValue ...

return m_records[idx];

Oh and that ; at the end of the for is not a mistake

To make sure people understand it you should either comment it, or put it
on the next line, or both. Alternatively, your code can be rewritten in
an easier to understand variation:

int idx = 0;
while (m_records[idx].GetValue() != iValue)
++idx;
return m_records[idx];

V

 

[Tobias Blomkvist]

Dylan sade:

Just wondered what you guys do when you need a method that returns a
const reference to some data based upon some value.

something like this:

Precondition:
m_iNumRecords contains an object whose GetValue() == iValue

const CData& MyClass::GetDataWithValue(int iValue)const
{
for (int idx=0; idx<m_iNumRecords; ++idx)
{
if (m_Records[idx].GetValue() == iValue)
{
return m_Records[idx];
}
}
}

If an event do arise where the precondition can't be secured, I would
consider this a dangerous function, well I do anyway. At least throw
an exception at the end to signal a precondition violation for
debugging purposes.

Most compilers will give a warning here. Something like "Warning: Not
all paths return a value"

This is fair enough but occasionally i know that this method will
*always* work correctly because there is no possible way iValue could
be anything else but valid. I'm certain this sort of thing crops up
for other programmers also. So my Q is: What do you do about it? (I
just ignore the warning at the moment but it irritates me!)

Even though I'm sure all data will always be valid in such a
case, I still don't blindly rely on it. A bug elsewhere can propagate
throughout the code structure and cause caos in other areas considered
secure due to naivity. Beware!

Tobias

 

[red floyd]

I'd optimize the loop if you can guarantee that you always get a
result.
in this case:

int idx;
for (idx =0; m_records[idx].getValue != iValue; ++idx);
return m_records[idx];

Oh and that ; at the end of the for is not a mistake

The way I prefer to write these is:

int idx;
for (idx = 0 ; m_records[idx].getValue() != iValue; ++idx)
/* DO NOTHING */ ;
return m_records[idx];

This way, it's clear that I explicitly intended an empty loop body,
and I don't have to add that parenthetical comment that you made.