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?
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?
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.
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.
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();
}
If you love us? You can donate to us via Paypal or buy me a coffee so we can maintain and grow! Thank you!
Donate Us With