Skip to content

Commit

Permalink
fix up apiaryio#7
Browse files Browse the repository at this point in the history
  • Loading branch information
alikh31 committed May 8, 2014
1 parent 423783d commit 4a637cf
Showing 1 changed file with 3 additions and 13 deletions.
16 changes: 3 additions & 13 deletions src/StringUtility.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,31 +22,21 @@
namespace snowcrash {

// Check a character not to be an space of any kind
inline bool isSpace(char i){
inline bool isSpace(const char i){

This comment has been minimized.

Copy link
@alikh31

alikh31 May 8, 2014

Author Owner

the reason of creating isSpace(const char i) rather than using std::isstring is due to a problem of software crashes on the debug mode, ofter parsing none ASCII code.

cc. @zdne

This comment has been minimized.

Copy link
@zdne

zdne May 8, 2014

Understood. Just a minor detail: Since this implementation is tied with std::string it might be a good idea to use const_reference e.g.: inline bool isSpace(std::string::const_reference i) or const std::string:: value_type&

This comment has been minimized.

Copy link
@alikh31

alikh31 May 8, 2014

Author Owner

@zdne: as we are just parsing the character through the function I don't think that it would be necessary to get the parameter by reference but I can change the type, do you want me to add the comment into the code too?

This comment has been minimized.

Copy link
@zdne

zdne May 8, 2014

Well one thing is to use reference (so the value isn't copied) but the good habit is to use iterator's value type anyway. In this case std::value_type, so if you decide in a future for example to change the type to wstring this is a obvious change.

This comment has been minimized.

Copy link
@alikh31

alikh31 May 8, 2014

Author Owner

the problem is when I use reference as input parameter I get this weird error: Error 2 error C2529: '_Left' : reference to reference is illegal and I cant figure out how to solve it. its something related to unary_function I guess.

This comment has been minimized.

Copy link
@zdne

zdne May 8, 2014

@alikh31 sorry hard to comment on this, since you do not provide the actual code. I believe you will figure it out.

This comment has been minimized.

Copy link
@alikh31

alikh31 May 8, 2014

Author Owner

seems like the problem is due to a problem on the vc compiler, it's again been suggested here to define the function as inline bool isSpace(const std::string::value_type i)

This comment has been minimized.

Copy link
@zdne

zdne May 8, 2014

Just out of curiosity – was this fixed in newer version of MSVC? Maybe we are really compromising here too much (supporting old MSVC). But again I leave the decision up to you.

This comment has been minimized.

Copy link
@alikh31

alikh31 May 8, 2014

Author Owner

seems like the problem is causing by the older version, but still as vc9 was a pretty stable and easy to use version problem such as this one would cause lots of confusion for MSVC2008 users.

if(i == ' ' || i == '\t' || i == '\n' || i == '\v' || i == '\f' || i == '\r')
return true;
return false;
}

// Trim string from start
inline std::string& TrimStringStart(std::string &s) {
std::string::iterator it;
for (it = s.begin() ; it < s.end() ; ++it){
if(!isSpace(*it))
break;
}
s.erase(s.begin(), it);
s.erase(s.begin(), std::find_if(s.begin(), s.end(), std::not1(std::ptr_fun(isSpace))));

This comment has been minimized.

Copy link
@zdne

zdne May 8, 2014

Looks much better!

return s;
}

// Trim string from end
inline std::string& TrimStringEnd(std::string &s) {
std::string::iterator it;
for (it = s.end()-1 ; it > s.begin() ; --it){
if(!isSpace(*it))
break;
}
s.erase(it+1 , s.end());
s.erase(std::find_if(s.rbegin(), s.rend(), std::not1(std::ptr_fun(isSpace))).base(), s.end());
return s;
}

Expand Down

0 comments on commit 4a637cf

Please sign in to comment.