Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

How to cast properly? (Or should I even do it?)

If I have code such as

class CString { int GetLength(); };

bool smaller(CString s1, std::string s2) {
    return s2.size() > s1.GetLength();
}

What is the best thing for me to do?

  • Change s1.GetLength() to (size_t)c.GetLength()?
    This would get help get rid of a compiler warning regarding "signed-unsigned mismatch", and communicate my intention to cast, and is by far the easiest route. But it's probably frowned upon. :(

  • Change s1.GetLength() to static_cast<size_t>(c.GetLength())?
    This would get help get rid of the warning, with "The Correct" kind of cast.

  • Change s1.GetLength() to static_cast<std::string::size_type>(c.GetLength())?
    It's extremely verbose... is there a practical benefit to this abstraction, or should I break it?

  • Leave it as is?
    This would help make the compiler do overflow-checking using the /RTCc switch (my main concern here), at the expense of a warning.

  • Do something else?
    Should I make my own casting function? Use a macro? Should I check at run-time as well as compile-time? Any other ideas?

Edit:

It seems like the example is being taken a little too literally...

I obviously didn't mean to talk just about CString::GetLength(). That particular method is certainly not a huge worry of mine. :) What I am worried about is the more general case, of when I'm getting an integer that's never supposed to be negative, but which could theoretically be, due to bugs.

Heck, I might be writing a method that does this, in order to override another piece of code -- so I can't change the signature. And my code could certainly have bugs, even though I wouldn't expect it.

In such a case, what should I do?

like image 295
user541686 Avatar asked Apr 05 '12 08:04

user541686


People also ask

What should you not do with a cast?

Avoid placing powder, lotion or deodorant on or near the cast. Leave adjustments to your child's doctor. Don't pull the padding out of your child's cast. Don't trim the cast or break off rough edges without first asking your child's doctor.


2 Answers

Can you change GetLength()? Fundamentally, the issue is that length is never negative, and an unsigned type reflects that the best. Length shouldn't be measured with an int.

But other than that, all three of your solutions are identical. std::string::size_type is always std::size_t, and while I would use a static_cast, the C-style cast performs the same cast, in this case. Because you know that the returned length is never negative (ensure this, by the way; you never know what weird things people might do), you're completely safe simply casting the type:

return s2.size() > static_cast<std::size_t>(s1.GetLength());

If CString::GetLength can be negative, for some reason, then it's up to you to decide how to make that conversion from negative to positive. Truncate? Magnitude (absolute value)? Whatever you need.


If you're worried about bugs, either do an explicit check and throw an exception (depending on your domain, this may be too costly), or use assert. Generally, though, you should trust the documentation.

like image 114
GManNickG Avatar answered Sep 21 '22 05:09

GManNickG


Put the cast in its own function, with a comment:

std::string::size_type size(const CString& mfcString)
{
    // CString::GetLength is always non-negative
    // http://msdn.microsoft.com/en-us/library/aa300471(v=vs.60).aspx

    return static_cast<std::string::size_type>(mfcString.GetLength());
}

Then your code will be:

bool smaller(const CString& s1, const std::string& s2)
{
    return size(s1) < s2.size();
}
like image 26
Peter Wood Avatar answered Sep 20 '22 05:09

Peter Wood