1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41
#define kSpace 032 char buf[] = {0x42, 0x73, 0x75, 0x27, 0x13, 0x1C, 0x68, 0x1B, 0x64}; #define mBufferLen(buf) (sizeof(buf) / sizeof(buf[0])) /* * convert the contents of 'buf' in place so that they are decoded. The * above buffer should convert to the ASCII string "Art&Logic" (but it doesn't as * presented here....) */ void Convert(char* buffer) { int i; for (i = 0; i <= mBufferLen(buffer); ++i) { if (buffer[i] > kSpace) { buffer[i] -= 1; } else { buffer[i] = buffer[i] << 2 + 4 - i; } } } int main(void) { int i; const char kTarget[] = "Art&Logic"; Convert(buf); for (i = 0; i < kBufferLen; ++i) { assert(kTarget[i] == buffer[i]); } }
Refactorings
No refactoring yet !
Andy Lester
October 14, 2007, October 14, 2007 17:00, permalink
I suspect you don't really want to be calling mBufferLen(buffer) every time through the loop. Assign it to a temporary variable first, and then use that variable in the loop condition.
Nathan Sanders
October 15, 2007, October 15, 2007 01:52, permalink
Look at the definition of kSpace. I think this is one of those 'change one character' puzzles.
Mike
October 15, 2007, October 15, 2007 11:12, permalink
Thanks for the "refactorings" so far. As for Andy's comment, yes, you're correct, I saw that, but I'm looking for more spec related issues than performance issues. Thanks again.
engtech
October 17, 2007, October 17, 2007 01:50, permalink
Step #1: hex vs dec... what is kspace in hex?
Step #2: put a comment beside buffer with the hex values of Art&Logic so you know what your goal is
I can see that your solution is wrong ampersand is greater than kSpace but it's only a -1 conversion.
1 2 3 4 5 6 7 8
#define kSpace 0x20 char buf[] = {0x42, 0x73, 0x75, 0x27, 0x13, 0x1C, 0x68, 0x1B, 0x64}; // http://asciichart.com/ascii_hexadecimal.html // A, r, t, &, L, o, g, i, c // 0x41, 0x72, 0x74, 0x26, 0x4c, 0x6f, 0x67, 0x69, 0x63 // -0x1, -0x1, -0x1, -0x1, +0x30, +0x03, -0x01, +0x4E, -0x01
engtech
October 17, 2007, October 17, 2007 01:51, permalink
if kSpace was supposed to be 0x32 how would that change your solution?
Mike
October 17, 2007, October 17, 2007 02:38, permalink
Thanks, engtech, that's exactly what I did in my solution. Again, I already "solved" it, I was simply told that the solution was "weak" on the verge of wrong; so I was looking for other unadulterated solutions as opposed to posting what I did.
jaredgrubb
October 29, 2007, October 29, 2007 15:44, permalink
A comment on Andy Lester's solution: There is no need to factor the condition check out of the for loop. mBufferLen(buf) is a macro function, so the compiler will text-replace the function code directly into the for loop check -- and the result will be exactly like a check against a constant.
Steve
November 5, 2007, November 05, 2007 20:54, permalink
#define mBufferLen(buf) (sizeof(buf) / sizeof(buf[0]))
This macro only works properly when used on arrays.
When used inside the function and called on the input parameter,
a pointer, it doesn't do what you want.
sizeof( char* ) / sizeof( char ) -> depends on the architecture, but probably 4.
The function needs to have a buffer size declared with it, and then you can call that function on the actual array from main.
Convert( buf, mBufferLen( buf ) );
You could avoid the buffer size requirement if there was some kind of string terminator to look for, but otherwise you need it.
Steve
December 19, 2007, December 19, 2007 15:38, permalink
buffer[i] = buffer[i] << 2 + 4 - i;
Additionally, << and >> bind last:
x = 1 << 2 + 3; is 32, not 7, your code should be rewritten as:
buffer[i] = ( buffer[i] << 2 ) + 4 - i; or
buffer[i] = buffer[i] * 4 + 4 - i; ... but you should use parenthesis to help make it more readable
chrome
January 11, 2008, January 11, 2008 23:34, permalink
What a horribly contrived "bug". No wonder most people hate C with lecturers like that.
Hi All,
I was given this code on a test. My solution was to pass a pointer to the buffer and adjust the shift values. I actually got the program to work according to spec (without changing the original input buffer), but was told that my solution was rubbish, so I'd appreciate another set of eyes to look at this and suggest their solution(s).
The requirements are as follows:
Fix the following piece of broken code. When executed, it should convert the buffer parameter (using the provided data -- changing the contents of the input buffer doesn't count as fixing a bug) into the string "Art&Logic".
Regards