The trim function and it's intented purpose?
in [General Programming]
|
Prev: CROSS: looking for someone who can translate my software interface to German, French and italian language
Next: How to keep a sorted list of 2D points in a loop?
From: Moi on 28 Mar 2010 14:07 On Sun, 28 Mar 2010 10:46:56 -0700, Lothar Behrens wrote: > On 28 Mrz., 19:27, Moi <r...(a)invalid.address.org> wrote: >> On Sun, 28 Mar 2010 09:45:47 -0700, Lothar Behrens wrote: >> >> > /** \brief Trim trailing spaces. >> > * >> > * Removes the trailing spaces in the string. */ >> > void LB_STDCALL lbString::trim() { >> > while (stringdata[strlen(stringdata)-1] == ' ') // stringdata is >> > of >> > type char* >> > stringdata[strlen(stringdata)-1] = 0; >> > } >> >> 1) What happens is strlen() returns zero ? > > Good point :-) > > void LB_STDCALL lbString::trim() { > if (stringdata == NULL) return; // Should never be if my test > case (correct initialization) passes. > if (strlen(stringdata) == 0) return; > while (stringdata[strlen(stringdata)-1] == ' ') // stringdata > is of type char* > stringdata[strlen(stringdata)-1] = 0; > } > > > >> 2) How many times will strlen() be called (supposed it _is_ a function) >> >> > It is a very dumb code. That's why I start creating unit tests. My code > works, but the point is, will it always work if other use my code? Well, the obvious way is of course: void mytrim(char *str) { size_t pos; /* it makes no sense to trim a NULL pointer */ if (!str) return; for (pos=strlen(str); pos--; ) { if (isspace( str[pos] )) str[pos] = '\0' ; else break; } } Note that strlen() is only called once. HTH, AvK
From: Lothar Behrens on 28 Mar 2010 14:12 On 28 Mrz., 20:07, Moi <r...(a)invalid.address.org> wrote: > On Sun, 28 Mar 2010 10:46:56 -0700, Lothar Behrens wrote: > > On 28 Mrz., 19:27, Moi <r...(a)invalid.address.org> wrote: > >> On Sun, 28 Mar 2010 09:45:47 -0700, Lothar Behrens wrote: > > >> > /** \brief Trim trailing spaces. > >> > * > >> > * Removes the trailing spaces in the string. */ > >> > void LB_STDCALL lbString::trim() { > >> > while (stringdata[strlen(stringdata)-1] == ' ') // stringdata is > >> > of > >> > type char* > >> > stringdata[strlen(stringdata)-1] = 0; > >> > } > > >> 1) What happens is strlen() returns zero ? > > > Good point :-) > > > void LB_STDCALL lbString::trim() { > > if (stringdata == NULL) return; // Should never be if my test > > case (correct initialization) passes. > > if (strlen(stringdata) == 0) return; > > while (stringdata[strlen(stringdata)-1] == ' ') // stringdata > > is of type char* > > stringdata[strlen(stringdata)-1] = 0; > > } > > >> 2) How many times will strlen() be called (supposed it _is_ a function) > > > It is a very dumb code. That's why I start creating unit tests. My code > > works, but the point is, will it always work if other use my code? > > Well, the obvious way is of course: > > void mytrim(char *str) > { > size_t pos; > > /* it makes no sense to trim a NULL pointer */ > if (!str) return; > > for (pos=strlen(str); pos--; ) { > if (isspace( str[pos] )) str[pos] = '\0' ; > else break; > } > > } > > Note that strlen() is only called once. > > HTH, > AvK Thanks
From: Patricia Shanahan on 28 Mar 2010 16:31 Moi wrote: > On Sun, 28 Mar 2010 10:46:56 -0700, Lothar Behrens wrote: .... >> void LB_STDCALL lbString::trim() { >> if (stringdata == NULL) return; // Should never be if my test >> case (correct initialization) passes. >> if (strlen(stringdata) == 0) return; >> while (stringdata[strlen(stringdata)-1] == ' ') // stringdata >> is of type char* >> stringdata[strlen(stringdata)-1] = 0; >> } .... > void mytrim(char *str) > { > size_t pos; > > /* it makes no sense to trim a NULL pointer */ > if (!str) return; > > for (pos=strlen(str); pos--; ) { > if (isspace( str[pos] )) str[pos] = '\0' ; > else break; > } > } These two functions have very different behavior. The first one only trims space, the second trims the characters that are considered to be whitespace according to the isspace function. Which behavior is intended? I think the second one would be more useful, but is it what the documentation says? Patricia
From: Moi on 28 Mar 2010 16:58 On Sun, 28 Mar 2010 13:31:48 -0700, Patricia Shanahan wrote: > Moi wrote: >> On Sun, 28 Mar 2010 10:46:56 -0700, Lothar Behrens wrote: > These two functions have very different behavior. The first one only > trims space, the second trims the characters that are considered to be > whitespace according to the isspace function. Which behavior is > intended? I think the second one would be more useful, but is it what > the documentation says? Well, it appears the comments said more than the code. (see may previous what/where comment) Which make us wonder. AvK
From: Thad Smith on 28 Mar 2010 20:55
Lothar Behrens wrote: > On 28 Mrz., 19:16, Ben Pfaff <b...(a)cs.stanford.edu> wrote: >> Lothar Behrens <lothar.behr...(a)lollisoft.de> writes: >>> I have implemented a trim function and documented it as follows: >>> /** \brief Trim trailing spaces. >>> * >>> * Removes the trailing spaces in the string. >>> */ >>> void trim(); >> [...] >>> What would you expect from this function? >> I would expect it to do what it is documented to do. > Yes, > > But with a trap I fell into (http://msdn.microsoft.com/de-de/library/ > system.string.trim(VS.80).aspx), I assumed a different behavior. Are you saying you wrote a function to remove trailing spaces, then assumed it did the same as someone else's trim function instead of what you implemented and documented? > I should indeed change the documentation, code and the test to match a > more common and known function :-) Whether you do that depends on your objectives. One of the problems here is using a very generic name (trim), which is subject to reuse for different purposes with the resulting confusion. Consider trimTrailingSpaces() for the function you originally described. Then document, code, test, and use correspondingly. -- Thad |