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: Ben Pfaff on 28 Mar 2010 13:16 Lothar Behrens <lothar.behrens(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. -- Ben Pfaff http://benpfaff.org
From: Moi on 28 Mar 2010 13:27 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 ? 2) How many times will strlen() be called (supposed it _is_ a function) > > I'll let my test fail by assuming it will do ltrim + rtrim and correct > the function after if I have more tests to ensure that changing this > function > will not break other code (TDD) :-) Well, Mybe you should create a test suite. in that case, don't forget to include the empty string as input. > > Also I haven't thought about the other cases (character set, backspace, > nonprintable chars, ...) IMHO you should. HTH, AvK
From: Lothar Behrens on 28 Mar 2010 13:34 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. > -- > Ben Pfaffhttp://benpfaff.org 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. I should indeed change the documentation, code and the test to match a more common and known function :-) Lothar
From: Thad Smith on 28 Mar 2010 14:38 Lothar Behrens wrote: > My implementation is a very dumb and easy one: > > /** \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; > } > > I'll let my test fail by assuming it will do ltrim + rtrim and correct > the function after if I have more tests to ensure that changing this > function > will not break other code (TDD) :-) This is backwards to me. Your test should, above all, verify that the function does what is claimed. Since your function only claims to "trim trailing spaces" that is exactly what the test should verify. Any other trimming is a failure of the function. If you want to write and test a different function, that's fine, but describe the intended operation and test for what is described. -- Thad
From: Lothar Behrens on 28 Mar 2010 13:46
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? I knew about unit tests a while, but not from the beginning. I hope to find much more code that is not well written :-) I now think different about the unit tests - they are important. > > > > I'll let my test fail by assuming it will do ltrim + rtrim and correct > > the function after if I have more tests to ensure that changing this > > function > > will not break other code (TDD) :-) > > Well, Mybe you should create a test suite. > in that case, don't forget to include the empty string as input. > I also have a problem with an uninitialized string (NULL) that will crash some functions. This test is added and keeps failing until I fix it - at next step. > > > > Also I haven't thought about the other cases (character set, backspace, > > nonprintable chars, ...) > > IMHO you should. > > HTH, > AvK |