Posts by arkoenig

You never change the value of your variable "current," so your program can never terminate.

Every time the compiler prints an error message during compilation, that message includes the number of the line with the problem.

If by "best" you mean the closest to 12 without being greater than 12, then what you have just described is a version of the knapsack problem, which is known to be hard to solve efficiently.

Use code tags. Without them, all your indentation goes away, which makes your code very hard to read.

If your book really contained the line

delete [] x,a,b,c,beta,gamma,r;

then I suggest you stop using that book, because the author doesn't know C++ very well. Ditto for the overall design of the program.

That said, the first place I'd look for your error is in the line

x(n-1)=gamma(n-1);

which surely should be

x[n-1]=gamma[n-1];

if that's not the only problem, please repost the code using code tags, and please tell us which line or lines the compiler said was in error.

Who knows? You certainly have not pasted all the relevant code, because you haven't told us what the value of the variable "i" is or what the noBucket function does.

I will say that your use of "i" makes me suspicious, because it's not a variable in your class. By implication, it's probably a global variable--what else could it be--and any program with a global variable named "i" is cause for suspicion all by itself.

If you are going to use the accumulate function to add floating-point numbers together, the third argument has to be 0.0, not 0

And if i understand you correctly then this additional exit condition would serve as an alternative break point from the inner loop after my current check?

No, instead of your current check. Once 2 is in the prime vector, the next number to be tested is 3. 2 * 2 is 4, which is strictly greater than 3; so you won't run off the end of the vector even when you're testing 3. Once you've gotten 3 safely onto the prime vector, 5 and subsequent primes are a piece of cake.

So here's one way you might write it:

// This assert should succeed because we pushed 2 onto primes
assert (!primes.empty());

primeList_t::iterator iPrime = primes.begin();
while (*iPrime <= limit && (x % *iprime) != 0) {
    ++iPrime;

    // This assert should work because the square of the last element
    // of primes is strictly greater than x, so the test *iPrime <= limit
    // should ensure that iPrime never referred to the last element of primes
    assert (iPrime != primes.end());

}

if (*iPrime > limit)
    primes.push_back(x);

Since when is 4 prime? Isn't that what you're claiming in line 20 of your code?

Also, if you care about execution time, one easy way to speed things up is to observe that you don't have to divide a candidate by every prime; just by the ones that are <= the square root of the candidate.

You have to be a little bit careful about this to ensure you don't inadvertently treat a perfect square as a prime because of rounding error in the square root, but an easy way to do that is compute the square root, round it, and then increment the rounded square root until its square is greater than the candidate. In other words:

unsigned long limit = sqrt(x);
while (limit * limit < x)
    ++limit;

Now you know that limit is >= sqrt(x) even if there are rounding problems, and you can use iPrime<=limit as the ending condition for your inner loop.

When you call getTile, you get a newly created value of type Tile. Your statement

project._p->it->getTile(x, y).CPosX = panel2->Location.X;project._p->it->getTile(x, y).CPosX = panel2->Location.X;

calls getTile to obtain such an object, then changes the CPosX member of that object, and finally throws the object away. Fortunately, C++ doesn't let you do that; because if it did, you'd never find your problem :-)

I am guessing that you want to make your getTile function return a Tile& rather than a plain Tile.

Hasn't he been dead over a whole year lol?

Rumors of my death are greatly exaggerated.

You're thinking of the Andrew Koenig who is the comedian who died last year, and who was the son of the original Star Trek actor Walter Koenig, who played Pavel Chekov.

No relation, so far as I know.

Now you've set month to 11, which means that your arrays have only 11 elements; the last element of averageStore is averageStore[10], and so on.

The right way to do this is to set your upper bound (i.e. month) to the number of elements you want, so

const int month = 12;

and then to exclude equality from your loops, such as

for (int i = 0; i < month; ++i) { /* whatever */ }

Personally, I prefer to use != instead of <, because doing so lets me use the same style for forward iterators, which do not support <; but I realize that not everyone agrees with me.

By the way, I don't understand why you're evaluating some array elements and then throwing them asay, as you do on lines 124 and 133.

If you want to ignore element number 0 in every array, you can certainly do that. It's not good style, but it can be made to work.

But in order to make it work, you have to make every array at least one element bigger than you think you need. In other words:

int foo[10];
for (int i = 0; i <= 10; ++i)
    foo[i] = 0;

This example never does anything with foo[0]. By itself, there's no harm in doing that. However, it also attempts to access foo[10], and that's wrong. There is no such element as foo[10], and there's no telling what the program will do when run.

So if you want to use this technique, at the very least you have to define foo to have 11 elements, not 10.

In the future, please post a complete program.

However.... Even though this program is incomplete, I can see a serious problem: In C++, arrays start with an index of 0. So when you write a loop such as

for(int i=1; i<=month; i++)	{
	monthAverage[i] = monthSum[i] / month;
}

that loop cannot possibly work: It ignores the first element (monthSum[0]) and tries to use a nonexistent element that is past the end of the array (monthSum[month]).

Here you go.

When you call a function foo with an argument x that is a variable of type char, you call it by writing foo(x) , not foo(char x) . That's your first problem.

Your next problem is that the variables d1 through d8 are not declared at the point at which you have called acknowledge_call .

And your third problem is that you are still not using code tags.

Please don't make us play guessing games: Show us exactly what you wrote, and tell us exactly what errors you got. And please use code tags.

You have defined acknowledge_call to take eight arguments. You have called it with no arguments. Hence the error message.

Next time, please use code tags.

Your dealCard member function has a serious flaw: It returns a reference to a local variable, which will refer to nothing valid after the function has returned. You should probably just return a Card (assuming that Card is a class whose values you can copy).

If you're going to use a vector to represent your deck of cards, you should really take cards off the end, not the beginning. The reason is that every time you take a card off the beginning, the library has to copy all the rest of the cards.

Also, I note in passing that before you remove a card from a deck, you should really verify that the deck is not empty.

You can implement these three suggestions by changing the dealCard member as follows:

Card Deck::dealCard()
{
    assert(!deck.empty());
    // Create temporary card
    Card temp = deck.back();
    // Take the card out of the deck
    deck.pop_back();
    return temp;
}

Did you call srand first?

What have you tried so far? Why doesn't it work? What sort of trouble are you having?

You can replace

for (unsigned long int k = 0; k < inTables[q].Items.at(j).Size; k++)			inTables[q].Items.at(j).Data.push_back(tempItem[k]);

with

inTables[q].Items.at(j).Data.insert(inTables[q].Items.at(j).Data.end(),
                                    tempItem,
                                    tempItem+inTables[q].Items.at(j).Size);

with the same effect. However, it would not surprise me to learn that doing so does not change the speed of your program much. Unless you have actually measured where it spends its time, you probably don't know where the time goes.

Here's a hint: You don't need an array.

Show us what you've done so far.

You tell me. An element of a string is a single character. It's not immediately clear what you mean when you say that a character is empty.

You said that TableRow is a vector<string>. Apparently it's just a string.

In line 1 of your code, you add an integer to what you claim is a vector. I don't know what you think that's supposed to do, but it's not well defined in standard C++.

Four times in you code, you explicitly convert an enum to an int. That shouldn't be necessary, and vector subscripts are unsigned anyway.

The most straightforward way to check whether a std::string object is empty is to call its empty member.

So... you should be writing something along these lines:

if(tableRow[H1280].empty())
    continue;

@DonutsnCode: Line 1 of your function says it returns int, not int&. It might still appear to work if you change it, but there's nothing wrong with the code you posted.

The paragraph you cite is an example of explaining how to avoid a bad programming technique by using a second bad programming technique. Here's why I say that.

A reference is an alternative name for an object. For example, if I write

int i;
int& j = i;

then I am saying, in effect, that j is an alternative name for i; in other words, that i and j are two different ways of referring to the same object.

When you write a function that returns a reference, that function is returning an alternative name for an object somewhere, because that is what a reference is. For such a function to be meaningful, it must return a reference to an object that will continue to exist after the function has returned.

So, for example, if I write the following:

int& foo()
{
    int n;
    return n;    // Don't do this!!
}

I have written a function that behaves in an undefined way when I call it. The reason is that if I write

int& x = foo();

I am saying that I want x to be an alternative name for whatever foo returns. However, what foo returns is an alternative name to its local variable n, and that variable ceases to exist when foo returns. Therefore, it returns an alternative name for something that does not exist, and x is similarly caused to name a nonexistent object.

The bad programming technique here is …

The curly braces are also part of C++0x. That's part of why I suggested that maybe your compiler doesn't support it yet--the C++0x standard isn't even finished yet.

Your declaration of "Item" is lower case here, though the class itself is uppercase.

auto Item * AoE2Wide= new Item(Pos, rf, "", "");

Does this work? Or am I misunderstanding your intention?

It's a C++0x usage. It is a request to declare a variable named "item" of the same type as that returned by the expression "new Item(<arguments>)"

Maybe your compiler doesn't completely support C++0x initializations yet.

What happens if you replace the curly braces with parentheses in the last line of code in your example, and change "auto" to "AoE2Wide::Item*" ?

The variable "last" you use in line 7 isn't defined, nor is it given a value anywhere.

If your list has only a single element, then presumably after you remove it, you need to set head to 0. Nowhere in the code is it possible for that to happen.

A somewhat easier way to write

copy(d1.begin(), d1.end(), d0);
copy(d2.begin(),d2.end(), &d0[d1.size()]);
copy(d3.begin(),d3.end(), &d0[d1.size()+d2.size()]);

is as follows:

double* next = copy(d1.begin(), d1.end(), d0);
next = copy(d2.begin(), d2.end(), next);
next = copy(d3.begin(), d3.end(), next);

After this code, next points to the first available location in d0.

Two points:

1) Why do you think your code was executed correctly? Is it not possible that it just happened to appear to be correct by coincidence? How would you go about proving that it was executed correctly even without a copy constructor.

2) If you have a copy constructor, you should probably also have an operator= member.

You've been studying this subject for two years. Surely you must be able to do something.

So how about showing us how much of your program you've written successfully. Then you might get a hint as to what to do next.

Are there any steps I can take while coding any function that will make it better?

Make it as simple as possible. That makes it easier to measure, and also easier to optimize the hot spots that the measurement reveals.

Step 1. Understand the problem thoroughly.

Step 2. Realize that I was really serious about step 1, and go back and understand the problem even more thoroughly.

Step 3. Write the clearest, most straightforward program you can that solves the problem correctly.

Step 4. Measure your program's performance. If you're satisfied with the results of your measurements, you're done.

Step 5. Your measurements will tell you what part(s) of your program are spending the most time. Rewrite those parts to make them faster. The most effective rewriting strategy will often be to find a way to avoid computing the same result more than once, or to find a more efficient algorithm. Now go back to step 4.

My experience is that many programmers will omit one or more of these steps in an effort to get the job done more quickly. Most of the time, doing so will have the opposite effect from what you intended.

I gave a new programmer's algorithm, one that will suffice for homework, not a mathematically perfect distribution. That is above the needs for a beginning programming class.

I completely disagree. As a practical matter, whether the original algorithm suffices for homework depends on the instructor. If I were the instructor, it certainly would not suffice.

As a philosophical matter, I think that as a general rule, it's a bad idea to advocate solutions that don't solve the stated problem correctly, and an even worse idea to do so without pointing out that the solution is oversimplified and doesn't actually yield the right answer.

As for the use of random_shuffle, I have three comments:

1) I didn't recommend it because I assumed that the OP would have used it if permitted.

2) Even if not, random_shuffle is geared toward linear data structures, not rectangular ones. It is true that you can treat an array of arrays as a linear data structure, but I believe that the subtleties of doing so are even greater than the subtleties of getting the code right in the first place. In fact...

3) The call to random_shuffle in the previous example is just plain wrong, for two reasons:

3a) The indices are wrong. If you want to use random_shuffle on the whole array, you have to pass an address that is one past the last element. That is &a[3][3], not &a[4][3]. Think about it.

3b) Unfortunately, even computing &a[3][3] …

b. “Algorithm P” which Knuth attributes to Moses and Oakford (1963), but is now known to have been anticipated by Fisher and Yates (1938); the 'Fisher–Yates shuffle' http://en.wikipedia.org/wiki/Fisher-Yates_shuffle

I believe the algorithm I suggested is equivalent to the one shown on this page under the heading "The Modern Algorithm."

Just some skeleton to help you visualize whats been said above :

for row = 0 to ROW_SIZE
    for col = 0 to COL_SIZE
         int rowToSwap  = a random valid row
         int colToSwap    = a random valid col
         swap array[row][col] with array[rowToSwap][colToSwap]

This is not quite right: You need to restrict the swapping so that the destination position is always >= the source position.

int rowToSwap = a random valid row that is >= row
int colToSwap = rowToSwap == row?
    a random valid row that is >= col :
    a random valid row

To see why, consider what happens if there are only two elements in the array using the original strategy. We look at the first element and swap it with the second with probability 0.5. Then we look at the second and swap it with the first with probability 0.5. The net effect is to keep the original ordering 3/4 of the time. Not a random distribution.

Take each element of your array in turn, and swap it with a randomly chosen element with a position that is >= the one you're swapping. Note that this raises the possibility that an element may be swapped with itself; if it is, that's equivalent to leaving that element alone.

It should be easy to see that after this is done, the first element can be swapped with any element in the array with equal probability. Applying this argument inductively shows that each element has equal probability of winding up in any spot in the array.

In the last section of code you posted, you are referring to DetectedFaces[j]. If TrackingFaces has more elements than DetectedFaces, at some point j will be >= DetectedFaces.size(). When that happens, DetectedFaces[j] is an out-of-range subscript.

You haven't told us the type of the elements of temp or DetectedFaces.

I see nothing technically wrong with the original code, which suggests that the problem is elsewhere.

However, the code is unnecessarily complicated and can also be very slow if the DetectedFaces vector is large. Moreover, it destroys the DetectedFaces vector, which may or may not be what you want.

A much easier solution:

temp.insert(temp.end(), DetectedFaces.begin(), DetectedFaces.end());

You're doing a division in line 8. Can you prove that the divisor is never zero?

Yeah but the function is recursive and is supposed to call itself until eventually it returns something.

Yeah but you're doing it wrong.

Every call, including recursive calls, must have a matching return. If any of these calls causes the "if" test in line 9 to be true, the corresponding return causes undefined behavior. The entire program could potentially do anything at all after that.

I really don't want to debate this. The code is broken, and there is no point in looking for any other problems until this one is fixed.

If the "if" statement that is the subject of the else in line 9 yields true in its test, then the function falls off the end without executing a return statement. The value that the function returns in that case is undefined.

I could tell you that functional programming (FP) is a style of programming in which one never changes a value that has already been computed. Instead, one creates new values as needed. By implication, using FP in practice requires a good garbage collector in order to get rid of values that are no longer needed.

Here's a simple example in C:

int factorial (int n) {
    int result = 1;
    while (n > 1) {
        result *= n;
        --n;
    }
    return result;
}

This code does not use FP, because it has the side effect of modifying the variables named n and result each time through the loop.

It is possible to achieve the same effect in an FP style, even in C, as follows:

int factorial(int n) {
    if (n <= 1)
        return 1;
    return factorial(n-1) * n;
}

You will see that in this second example, no variable is ever changed after being given its initial value, so there are no side effects.

The foregoing explanation is grossly oversimplified, and I can think of no short way to explain why FP is important or useful, but this is a start.

As for your other questions:

There are several programming languages in widespread use that make FP easy (or sometimes even necessary). The three best known are probably Lisp (particularly its Scheme dialect), Haskell, and SML. There are substantial differences between the three languages, but it's not easy to explain those difference …

1. I see nothing wrong with your code.
2. Yes.
3. It's up to the compiler whether a1 and a2 share the same string constant.

Line 27 of your original code sets the variable sizeA to 0. Line 30 calls the function getarrays, passing a reference to sizeA. Nowhere in the getarrays function do I see any code that ever changes the value of sizeA.

So on return from getarrays, sizeA is still 0. I doubt that's what you have in mind.

The same reasoning applies to sizeB.

I suggest you break this program into smaller pieces and test each piece before you try to combine them.