patterncppMinor
Checking if a string is a number with STL
Viewed 0 times
numberstlwithcheckingstring
Problem
Recently me and colleague had a discussion about the following piece of code (simple bool function that checks if string is a number,
He felt that it was hackish, while I thought it was nice, clean and elegant (since it uses STL, not hand made loops).
So is this a judgement call or one of us is wrong? Is this code elegant or hack?
+1000 not allowed, -1000, 1234 ... allowed). He felt that it was hackish, while I thought it was nice, clean and elegant (since it uses STL, not hand made loops).
So is this a judgement call or one of us is wrong? Is this code elegant or hack?
bool validate(const std::string& m)
{
if (m.empty())
return false;
return m.end() == find_if(*m.begin() == '-'? ++m.begin() : m.begin(),
m.end(), not1(ptr_fun(isdigit)));
}Solution
I had to really look at your code and think about what it was doing before I figured it out. That tells me that this code is not elegant by my definition of "elegant". My definition of elegant is something like,
Easy to understand, easy to maintain, efficient in execution, robust.
I would suggest something like this as a start:
Not much caring for the use of
I find this to be much more elegant than the 1-liner you wrote. It's more code, sure. But it is exceedingly simple to understand and maintain. You found a bug yourself just by brain-compiling it. That's what code should look like.
Easy to understand, easy to maintain, efficient in execution, robust.
I would suggest something like this as a start:
if( m.empty() )
return false;
string::iterator start = m.begin();
if( *start == '-' )
++start;
if( m.end() == find_if( start, m_end(), not1(ptr_fun(isdigit)) ))
return true;
else
return false;Not much caring for the use of
find_if, I'd refine it to:if( m.empty() )
return false;
string::size_type pos = 0;
if( m[0] == '-' )
pos = 1;
if( m.find_first_not_of("0123456789", pos) == m.end() )
return true;
else
return false;I find this to be much more elegant than the 1-liner you wrote. It's more code, sure. But it is exceedingly simple to understand and maintain. You found a bug yourself just by brain-compiling it. That's what code should look like.
Code Snippets
if( m.empty() )
return false;
string::iterator start = m.begin();
if( *start == '-' )
++start;
if( m.end() == find_if( start, m_end(), not1(ptr_fun(isdigit)) ))
return true;
else
return false;if( m.empty() )
return false;
string::size_type pos = 0;
if( m[0] == '-' )
pos = 1;
if( m.find_first_not_of("0123456789", pos) == m.end() )
return true;
else
return false;Context
StackExchange Code Review Q#12959, answer score: 4
Revisions (0)
No revisions yet.