Sign in to follow this  
Charabis

XCode Errors

Recommended Posts

Okay, trying to compile a program, but XCode is going really screwy . First, the code snippet:
	if (direction)
	{
		switch (rot)
		{
			case 0:
                    {
                        Point2i temp = p1;
						p1 = p2;
						p2 = p3;
						temp.setx (temp.getx() + TILESIZE);
						p3 = temp;
						rot = 3;
						break;
                    }

			case 1:		p3.sety (p3.gety() - TILESIZE);
						rot = 0;
						break;

			case 2:		p2.setx (p2.getx() - TILESIZE);
						rot = 1;
						break;

			case 3:		p1.sety (p1.gety() - TILESIZE);
						rot = 2;
						break;
		}
	}
	else
	{
		switch (rot)
		{
			case 0:		p3.sety (p3.gety() - TILESIZE);
						rot = 1;
						break;

			case 1:		p2.setx (p2.getx() + TILESIZE);
						rot = 2;
						break;

			case 2:		p1.sety (p1.gety() + TILESIZE);
						rot = 3;
						break;

			case 3:		Point2i temp = p3;
						p3 = p2;
						p2 = p1;
						temp.setx (temp.getx() - TILESIZE);
						p1 = temp;
						break;
		}
	}
On the first switch, it chokes on the 'case 0:' line, complaining that
Quote:
error: crosses initializtion of 'Point2i temp'
Not only that, but that one error is reported 3 times, along with this error on each of the following case lines within that section of the if.
Quote:
error: jump to case label
Oddly, the else portion works fine as is. I did find these could be eliminated simply by declaring the variable BEFORE the switch, although that means it will be created EVERY time it's called instead of in just the one case where it's needed. To add insult to injury, if I change it to the way it will work, it then throws me this error:
Quote:
Undefined symbols: _TILESIZE
Which makes no sense, considering:
const extern int TILESIZE;
is in the Tring.h header file it includes, along with the definition
const int TILESIZE = 10;
in main.cpp So what the heck exactly is going on? At this point, I'm at a complete loss. Not to mention it's a heck of a way to start my second day of working with XCode [razz] [Edited by - Charabis on March 8, 2007 4:31:44 PM]

Share this post


Link to post
Share on other sites
Well to fix your first error, without having to move the Point2i outside of the switch statement.


case 0:
{
Point2i temp = p1;
p1 = p2;
p2 = p3;
temp.setx (temp.getx() + TILESIZE);
p3 = temp;
rot = 3;
break;
}



just add curly braces like so.

Share this post


Link to post
Share on other sites
That did indeed solve the first problem, although I'm not completely sure why.

I went back and changed the appropriate quotes to code and source, but it didn't help with either problem [lol]

Share this post


Link to post
Share on other sites
Well, you should also learn how to use whitespace in your code. :)

While you can't type a tab into the web editor, you can paste chunk of code that contains tabs.

These are things that help people (including you) read your code.

Do you understand indentation styles, or is it just a web formatting problem?

Quote:
That did indeed solve the first problem, although I'm not completely sure why.


The variable "temp" was visible in every case of your switch statement. This is illegal in C++. The squiggly braces block prevents the problem.

Share this post


Link to post
Share on other sites
It's a problem with switch statements, by adding the curly braces you force that variable into its own scope within the case, so when it leaves the curly braces that variable is destroyed.

Share this post


Link to post
Share on other sites
Actually, it looked a lot better when I started, Notayakk. Problem is I pasted the code into the Mac forums trying to find an answer, and it stripped out all my whitespaces. Feeling somewhat lazy, I just copied and pasted from there. Didn't even notice it had stripped all my whitespace out. I'll edit real quick.

EDIT : Although it's still a bit off on the alignment from what I show in my editors. Guess that's what I get for using the tab key to align my expressions [grin]

Share this post


Link to post
Share on other sites
I now have a limited fix for the variable.

const int TILESIZE = 10;


If I pull the 'const', I can get it to pull the variable into the other files. Can constants not be pulled into a file using the 'extern' keyword?

Share this post


Link to post
Share on other sites
const int TILESIZE = 10;
means
static const int TILESIZE = 10;

ie, const variables in C++ are implicitly of internal linkage.

The syntax to export a const int is:
extern const int TILESIZE = 10;
however, I advise against doing this, even though it fixes your problem.

...

Practically, you should simply move the
const int TILESIZE = 10;
into the header file, instead of "extern const int TILESIZE" -- it will have static linkage, and be turned into a constant wherever it is used and the optimizer works on it.

Share this post


Link to post
Share on other sites
Another code-style comment.

You might want to write your switch/case statements as follows:


switch(x) {
case 1: {
printf("%s\n", "X has value 1");
} break;
case 2: {
printf("%s\n", "X has value 2");
} break;
case 3:
case 4: {
printf("%s\n", "X has value 3 or 4");
} break;
default: {
printf("%s\n", "X is a chicken");
} break;
}


It does a few things: it restricts the scope of variable declairations to be within one case label, and the visual pattern makes it clear that "rollthrough" won't be happening.

Next, a cute trick to create "real integer constants" is to use an anonymous enum:


enum { TILESIZE=10 };


This has the wonderful property that you can't take the address of an enum token (and then accidentally change it!). :) Plus, the size of a tile becomes "truely compile-time" (it is allowed in some different syntaxes than a static const int is).

Share this post


Link to post
Share on other sites
Quote:
Original post by NotAYakk
It does a few things: it restricts the scope of variable declairations to be within one case label, and the visual pattern makes it clear that "rollthrough" won't be happening.


It also lets you keep indentation and scope matched.

I find it extra work, in normal cases, though (needing to declare a variable in order to handle a case of a switch suggests that refactoring is needed, anyway). So I normally just don't indent my case blocks. :P

Quote:

Plus, the size of a tile becomes "truely compile-time" (it is allowed in some different syntaxes than a static const int is).


Um, the OP is using C++ (a .cpp file is explicitly mentioned, and a struct is assigned directly), and unless you can think of a syntactical corner case that I can't, that only applies to C.

Anyway, some (OK, a LOT, because I always end up obsessing...) refactoring notes.

0) We don't need the temp variable anyway. Just increment p1 before doing the 3-way swap, and you get the same net effect.

1) We have several variables named with numbers at the end, which are handled similarly. This begs for an array to be used instead.

2) We can extract a function to do the 3-way swap, and improve code reuse immediately (and probably more in the future). (See also (0)). Except that we don't even need to do that, because we're putting the variables into an array (see (0) again ;) ), and the standard library provides a function to "rotate" arrays (actually, to rotate iterator-ranges - a more powerful concept).

3) Accessor/mutator pairs smell. Accessor/mutator pairs that commonly get used in pairs smell horribly. What we want to do is add a Point2i member function that increments the x, rather than setting it directly. In fact, instead of doing that separately for x and y, it makes more sense to make a function that takes in a delta-x and delta-y. There's a perfectly good name for it, too: move().

FWIW, I have seen this *exact* example *way more times than I want to count*. ;)

4) The 'rot' (short for 'rotation' I assume? Would it kill you to type the rest?) value gets, well, rotated in every case; we can handle that with a little arithmetic instead.



#include <algorithm> // where std::rotate lives
// Doc: http://www.sgi.com/tech/stl/rotate.html

// We'll need to add:
void Point2i::move(int dx, int dy) {
m_x += dx; m_y += dy; // or whatever you called the members...
}
// And in fact, *remove* those set/get x/y functions.

// We'll put p1, p2, and p3 in order into an array p of size 3. Then...
if (direction) {
switch (rotation) {
case 0:
p[0].move(TILESIZE, 0);
std::rotate(p, p + 1, p + 3);
break;

case 1: p[2].move(0, -TILESIZE); break;

case 2: p[1].move(-TILESIZE, 0); break;

case 3: p[0].move(0, -TILESIZE); break;
}
rotation = (rotation + 3) % 4; // the math I was talking about.
// This is a quite common trick.
} else {
switch (rotation) {
case 0: p[2].move(0, -TILESIZE); break;

case 1: p[1].move(TILESIZE, 0); break;

case 2: p[0].move(0, TILESIZE); break;

case 3:
p[2].move(-TILESIZE, 0);
std::rotate(p, p + 2, p + 3);
break;
}
if (rotation != 3) { rotation = (rotation + 1) % 4; }
}



Looks a lot better, right? But if you think I'm done, you're quite wrong. :)

First of all, you'll note that I've been careful to preserve the original behaviour. This highlights a few apparent bugs - or at least, I can only assume the lack of symmetry wasn't intentional:

- in the !direction, rotation = 3 case, we don't adjust rotation afterwards - even though we do when direction = true, rotation = 0.

- There's no case where we move(0, TILESIZE) if direction, and a duplicate case where we move(0, -TILESIZE). My Jedi mindreading tells me that you want to move(0, TILESIZE) in case 3. :)

But we can still do better than "just" fixing the bugs, because suddenly the switch statements all look very much alike. To avoid repetition, what I'll do is put the possible dx and dy values into arrays, and use the case values to index into the array. Then we handle the rotation requests separately.


#include <algorithm>

void Point2i::move(int dx, int dy) {
m_x += dx; m_y += dy; // or whatever you called the members...
}

int rotation_dx[4] = { TILESIZE, 0, -TILESIZE, 0 };
int rotation_dy[4] = { 0, -TILESIZE, 0, TILESIZE };

// We'll put p1, p2, and p3 in order into an array p of size 3. Then...
if (direction) {
// Again we can observe the pattern and apply some math to figure out
// which point to move...
p[(3 - rotation) % 3].move(rotation_dx[rotation], rotation_dy[rotation]);
if (rotation == 0) { std::rotate(p, p + 1, p + 3); }
rotation = (rotation + 3) % 4;
} else {
p[(5 - rotation) % 3].move(rotation_dx[(5 - rotation) % 4], rotation_dy[(5 - rotation) % 4]);
if (rotation == 3) { std::rotate(p, p + 2, p + 3); }
rotation = (rotation + 1) % 4;
}



And I hope you don't imagine I'm done now, because I'm just about to add water to the jar of rocks and sand. :)

- We've got parallel arrays for the rotation offsets; would make more sense to have points instead, and a move(Point2i&) function instead.

- Both if-cases have the same form; we can use the direction instead to initialize a few constants, and then use common code. (This will give us a chance to give meaningful names to some of those weird modulo-expressions, which helps move towards self-commenting code. Normally I don't like variables that are only read once and written once, but I'll admit the math is tricky and needs some explanation now :) )

- Our math gets simplified if we notice that the cases are "backwards" of each other depending on the value of direction. Therefore, we can simplify our math by, in the !direction case, "flipping" the rotation value (x = 3 - x) before and after. We also notice that when we do that, our index into the rotation directions becomes 2 + what it would have been (mod 4 of course), but we also see that we can get the same effect by negating the rotation direction. So, we add a unary negation operator to the Point2i, and we're good to go. We also do the same math to "rotate the rotation state" in both cases because of this transform.

- We can apply the same trick to our pointToMove calculation to simplify it - we factor the calculation down into a flipped or unflipped value of rotation, with a possible second flipping.

- Actually, we shold rotate when direction is *true*, since it removes an extra flip used in the pointToMove calculation, as we see once it's factored like that. To implement that I had to flip the rotation-directions array too.

This is the final result. Please check that I didn't make any mistakes ;)


#include <algorithm>

void Point2i::move(const Point& distance) {
m_x += distance.m_x; m_y += distance.m_y;
}

Point2i Point2i::operator-() {
return Point2i(-m_x, -m_y);
}

Point2i rotationDirections[4] = {
Point2i(0, TILESIZE), Point2i(-TILESIZE, 0),
Point2i(0, -TILESIZE), Point2i(TILESIZE, 0)
};

if (direction) { rotation = 3 - rotation; }

int pointToMove = rotation % 3;
if (!direction) { pointToMove = 2 - pointToMove; }
Point2i movement = rotationDirections[rotation];
p[pointToMove].move(direction ? movement : -movement);
if (rotation == 3) { std::rotate(p, direction ? (p + 1) : (p + 2), p + 3); }
rotation = (rotation + 1) % 4;

if (direction) { rotation = 3 - rotation; }

Share this post


Link to post
Share on other sites
Wow [smile] Now I can't wait till I get home to mess with some of that, but I can comment on a few things even from work (Assuming nobody calls in the next few minutes [razz])

Guess I should have explained exactly what it is I'm doing for clarity. Bit of an 'oops' there on my part, as I'm doing something very simplistic even though it might not look that way.

I'm using the structure to hold triangles that can be rotated. Since I wanted them to remain within the same square region, I plotted the 4 points and noted where each one would wind up plotted.

Quote:

1,2,3 for base (0)
1,2,4 for 1st clockwise
1,3,4 for 2nd
2,3,4 for 3rd


and designed from there. I was trying to keep things simple at this point with no optimizations since this would be my first graphical program, and the C++ ones in C++ Primer Plus, while educational, are hardly impressive constructs [razz]

Of course I was going to expand from there, and, in fact, already did far ahead of when I had planned for it. I discovered SDL_Image in the meantime and switched all my backgrounds over to png to save some space, but I digress.

You've definitely given me some food for thought on this, but I'll have to eat it later [grin] It took over 15 minutes to get this written, so it's looking to be a busy day at work. No time for playing with code today! Only 6 hours to go!

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this