Programmer Test

I am not sure, but wouldnt the language be c++ (in ref to that source lang thing, or is that a note to the syntax highlighter?) Last time I checked (which was a while ago) C did not allow for(int a...). You need to declare i out of for. The other thing is you could make your description more specific. It uses random angles. However I doubt either of those things have much to do with it. Perhaps you could use sin/cos tables.

Well the really smelly error in that code is the macro CALC_PNT which is unsafe beacuse it references a variable twice (edit: and also because it contains more than one statement).

Each time the macro is called in the example function it calls rand_FloatRange(0.0f, 2.0f*PI) once for x and then again for y giving you a not entirely random distribution of points within a square with sides equal to 20 centred on (0,0).

The macro should be rewritten as an inline function.

macros are evil. *click*

You could also unroll the loop to optimise. I only glanced at it. There may be other errors / optimisations. I wouldn't have changed the name of the function in case it was already in use in a library somewhere.

One problem is that sin, cos return a double, and so you would need a typecast in there. You would want to get rid of the macros, for clarity. math.h already has a #define for pi.

These tests are silly, style really depends on the person. Some people get upset if you have more than one return in a function, a goto or break, etc,etc...

I recommend revising the following:

basic trigonometry
basic newtonian motion physics
basic vector and matrix mathematics
coordinate space transformations
bit shifting operations and related optimisation tricks
common C / C++ errors

Know your basic formulas for this stuff in your head.
Practice so you become fast.
Practice time management during tests (productivity is very important).

Quote:

Original post by ironpoint These tests are silly, style really depends on the person. Some people get upset if you have more than one return in a function, a goto or break, etc,etc...

This is not how we get jobs in commercial game development.

Quote:

Original post by Geocyte Each time the macro is called in the example function it calls rand_FloatRange(0.0f, 2.0f*PI) once for x and then again for y giving you a not entirely random distribution of points within a square with sides equal to 20 centred on (0,0). The macro should be rewritten as an inline function. macros are evil. *click*

I pray that this was the point of the exercise. If not, then something tells me that isn't a firm you want to work at anyhow. A third issue with that macro is that it references x and y, variables it shouldn't even know exist if a casual reader assumes its a function.

I wouldn't have moved x and y outside the for loop...they are unneeded elsewhere, so they belong within the loop. And in this case, ++i and i++ are always the same in C, so making that change for efficiency is a wasted effort. The only change I see being needed is to make that macro a function [that generates two random angles, with a note explaining why] and give it a proper name like CalcPntOnSquare.

CM

Quote:

Original post by Witchcraven I am not sure, but wouldnt the language be c++ (in ref to that source lang thing, or is that a note to the syntax highlighter?) Last time I checked (which was a while ago) C did not allow for(int a...). You need to declare i out of for.

You are thinking of a VC++ 6.0 problem, not a C problem. You can put the variable definition in the for in VC++ 6.0, but it will have problems on code like this:

for (int i = 0; ...; ...) {     ...}for (int i = 0; ...; ...) {    // It will signal a multiple definition error, because the <tt>for</tt> scoping is messed up.     ...}

Quote:

Original post by Geocyte I recommend revising the following: basic trigonometry basic newtonian motion physics basic vector and matrix mathematics coordinate space transformations bit shifting operations and related optimisation tricks common C / C++ errors Know your basic formulas for this stuff in your head. Practice so you become fast. Practice time management during tests (productivity is very important).

*slapping myself* Thanks Geo, i should've known to scrutinize the macro. BTW, i enjoyed talking to your president and i wish you guys the best.

Quote:

Original post by Conner McCloud I wouldn't have moved x and y outside the for loop... CM

Isn't that more efficient than allocating stack space for every iteration of the loop?

Quote:

Original post by Conner McCloud A third issue with that macro is that it references x and y, variables it shouldn't even know exist if a casual reader assumes its a function.

Ah, yes. I left that one as an exercise for the reader. ahem. *blushes*

Quote:

Original post by Conner McCloud The only change I see being needed is to make that macro a function [that generates two random angles, with a note explaining why] and give it a proper name like CalcPntOnSquare. CM

Once it's working properly it will calculate points on a circle :)

Quote:

Original post by Mr Furious Quote: Original post by Conner McCloud I wouldn't have moved x and y outside the for loop... CM Isn't that more efficient than allocating stack space for every iteration of the loop?

Not really. A decent compiler should optimise that correctly so it's best to declare variables in the most specific scope possible in order to maximise readability.

I would have done it like so:


Although i made an assumption that CALC_PNT isn't used elsewhere because it references specific variables.

if i didn't make that assumption, i'd have made it an inline function as stated above.

I think one person here really touched on what the interviewer was most interested in seeing in there. The interviwers almost certainly wanted to see this macro changed:

#define CALC_PNT(rad, ang) x = rad*cos(ang); y = rad*sin(ang);

to:

#define CALC_PNT(rad, ang) x = rad*cosf(ang); y = rad*sinf(ang);

As someone else already pointed out, the standard C runtime functions sin() & cos() accept and rerturn doubles. The functions sinf() and cosf() accept and return floats.

This really depends on the coordinate system, but there are cases where 100 points would completely fill the circle. There are better and much faster ways to render circles. And finally, the original code only draws one specific circle, so you could perhaps just blit precomputed image instead.

Quote:

Original post by ironpoint math.h already has a #define for pi.

In VC++ 6.0 it doesn't. However, you probably could find a better definition than just 3.14159f. We have used "double pi = 2.0*asin(1.0);" for that.

Since all the trig functions use doubles pi should aswell IMHO.

Quote:

Original post by Witchcraven I am not sure, but wouldnt the language be c++ (in ref to that source lang thing, or is that a note to the syntax highlighter?) Last time I checked (which was a while ago) C did not allow for(int a...). You need to declare i out of for. ...

In C99, "for(int i=0;..." is permissable.

Quote:

Original post by Mr Furious I was recently given this problem from a test for a company in southern california: *** Source Snippet Removed *** My answer was this: *** Source Snippet Removed *** However, this answer was not satisfactory. What more can you do?

Well, for one thing you removed the "CALC_PNT" but still reference it.

Aside from some other issues mentioned in the thread...

Quote:

Original post by Troll I think one person here really touched on what the interviewer was most interested in seeing in there. The interviwers almost certainly wanted to see this macro changed:

No, the one problem with the code is that it is totally broken due to the incorrect use of a macro that references an argument twice.

When a macro references an argument twice, then that argument will be evaluated twice. That means: The random function gets called twice for every point. Ergo, the code is just plain WRONG.

So, well done Geocyte (and Mr. McCloud) for noticing that, and shame on everyone else (including myself, I have to admit)

John B

Quote:

Original post by BorisTSR I would have done it like so: const float PI 3.14159f; //defines are evilvoid PlotPnts(){ float radius = 10.0f; // store the value once float x,y,ang; // to prevent constant allocation and freeing of memory for(int i = 0; i < 100; i++) { ang = rand_FloatRange(0.0f,2.0f*PI); x = rad*cos(ang); y = rad*sin(ang); plot_point(x,y); }}

Just to point out that any non-obsolete compiler would not constantly allocate and free stack variables inside the loop. Furthermore, any compiler that would constantly readjust the stack in that manner would almost certainly also fail to perform proper liveness analysis on your version and therefore have three extra variables to store throughout the lifetime of the function, potentially causing the values to spill out of registers and require writing to main memory anyway. The moral of the story? Declare variables as late as you can.

Enigma

Just my 2 cent

I guess this little programm is only drawing the points for the outline of a circle, not a filled circle?
I haven't seen it in any of your answers if you mean the draw a filled circle or the outline anywhere...