C++ / Logic Question...

[JSThePatriot]

Little Background:

At my company, I fix/update the current software offering we have in place. I have taken the code and tried to shorten it down, and make it more readable, and many other things to help our software run better, and be easier to use and maintain.

This first code segment is the "old" and "bloated" manner of doing this. (iii, iio, iio2, PieceNo, and MaxPiece[] are integers | values[][] is a CString array array | buffers[] is a char array | fp is a FILE pointer)

for (PieceNo = 0; PieceNo <= MaxPiece[iii]; PieceNo++)
{
    values[iii][PieceNo] = _T(",") + values[iii][PieceNo];

    for (iio = 0,iio2 = 0; iio < values[iii][PieceNo].GetLength(); iio++)
    {
        if (values[iii][PieceNo][iio] > 0x19)
        {
            buffers[iio2++] = static_cast<char>(values[iii][PieceNo][iio]);
            buffers[iio2+1] = 0;
        }
    }

    fputs( buffers, fp );
}

fputs( "\n", fp );

The second segment of code is supposed to do the same thing, but without the "conversion" from CString to char. Does the logic look the same for the two methods?

for (PieceNo = 0; PieceNo <= MaxPiece[iii]; PieceNo++) {
    _ftprintf(fp,_T(",%s"), values[iii][PieceNo]);
}

fputs( "\n", fp );

Thanks for any help or input on the situation.

JS

[Valik]

Absolutely not. The original code strips characters 0x00 - 0x19. In the original code, strlen(buffers) may not be the same as values[iii][PieceNo].GetLength(). Your new code just dumps the entire string. Thus, your code has changed the behavior.

You can use fputc() and you won't need the char[] buffer with that as you can just output each character after you test that it's > 0x19. However, keep in mind, the code you are claiming is bloated may just be optimized. It could be that fputs() is faster than multiple calls to fputc() - and it almost certainly is. So the question you need to ask is this: Is the memory you're losing by keeping everything in that extra char[] buffer more important than the CPU cycles you're saving by not repeatedly calling fputc() every time? And, of course, actually test to see which way is faster if the performance is important.

Edit: Added some stuff, changed some stuff.

Edited September 5, 2008 by Valik

[JSThePatriot]

Absolutely not. The original code strips characters 0x00 - 0x19. In the original code, strlen(buffers) may not be the same as values[iii][PieceNo].GetLength(). Your new code just dumps the entire string. Thus, your code has changed the behavior.

You can use fputc() and you won't need the char[] buffer with that as you can just output each character after you test that it's > 0x19. However, keep in mind, the code you are claiming is bloated may just be optimized. It could be that fputs() is faster than multiple calls to fputc() - and it almost certainly is. So the question you need to ask is this: Is the memory you're losing by keeping everything in that extra char[] buffer more important than the CPU cycles you're saving by not repeatedly calling fputc() every time? And, of course, actually test to see which way is faster if the performance is important.

Edit: Added some stuff, changed some stuff.

Valik,

Thank you for pointing out these points. I did know my code doesn't strip those characters. The reason he is "stripping" them is due (my assumption) to not wanting non-printable characters in a string that is to be written to file. Those characters shouldn't be in my string at all. 0x00 was in his string due to it being a char array. Also you are correct about the strlen(buffers) doesn't exactly = values[iii][PieceNo].GetLength(), but that I believe is the difference in the fact that CStrings are Unicode compliant, and strlen() isn't. (I could be wrong on that...)

The reason why he cycles through every character is because he didn't know how to write a "string" to file. He does these conversions throughout the code which ends up adding many lines that seem pointless to me. I have been working with this code for well over a year trying to figure out different things that he has done. There aren't many comments or other explaining items. Everything I have had to figure out on my own.

Again if he is just simply converting from a CString to char array since he didn't know how to write a CString to file, the character by character and performance items mentioned above are mute as that isn't necessary.

Any more comments or ideas fully welcome!

Thanks again for the above,

Jarvis

[Valik]

Valik,

Thank you for pointing out these points. I did know my code doesn't strip those characters. The reason he is "stripping" them is due (my assumption) to not wanting non-printable characters in a string that is to be written to file.

There are more non-printable characters than just those. You can look at the appendix of ASCII characters in the AutoIt documentation to see those that don't print (Most of the character from 0x00 - 0x31 do not print). The code also strips out tabs and newlines, by the way.

Those characters shouldn't be in my string at all. 0x00 was in his string due to it being a char array.

Are you certain they can't appear? Is input validated somewhere else?

Also you are correct about the strlen(buffers) doesn't exactly = values[iii][PieceNo].GetLength(), but that I believe is the difference in the fact that CStrings are Unicode compliant, and strlen() isn't. (I could be wrong on that...)

String length functions return the number of characters, not the number of bytes. My point in saying that was, his output may not be entirely the same as his input because he strips characters.

[JSThePatriot]

There are more non-printable characters than just those. You can look at the appendix of ASCII characters in the AutoIt documentation to see those that don't print (Most of the character from 0x00 - 0x31 do not print). The code also strips out tabs and newlines, by the way.

My mistake. You are correct on that. I thought I had looked it up previously.

Are you certain they can't appear? Is input validated somewhere else?

To my knowledge, input isn't validated. It is being pulled in through a GetWindowText() function. I realize this doesn't validate, it but for some reason I had it in my little head that text boxes couldn't accept all those characters. We accept items to the text box through 3 different methods... serial strings (com port), direct entry by the keypad, and by a scanned bar code.

String length functions return the number of characters, not the number of bytes. My point in saying that was, his output may not be entirely the same as his input because he strips characters.

That's true, and another thing I have to think of moving forward is that we're looking to internationalize our application, and with simple char array's and character by character conversions to and from a CString doesn't seem like it would be plausible to internationalize. I am just trying to make sure that I do the code properly. There are many times in the application he uses similar for loops with no ifs on the characters, just to convert it to a char array, which is why I thought I could easily change this over.

Thanks,

Jarvis