/= doesn't do what you think it does?

**laserlight** · 09-21-2008

And k only looped from zero to one.

That's right, so my example is not correct.

In my defense, we're looking at code that has undescriptive variable names and magic numbers, with a complex mix of data structures that really should be simplified with the use of classes.

**elninio** · 09-21-2008

Originally Posted by iMalc

Yep, it looks to me like that explains the problem too.

kFmap is a vector of maps:

vector<map<string,int> > kFmap

Its elements are defined for the following:

kFmap[0][1]
kFmap[0][2]
kFmap[0][3]
kFmap[0][4]
kFmap[1][1]
kFmap[1][2]
kFmap[1][3]
kFmap[1][4]

In your argument your saying I'm accesssing kFmap[2, 3] , thats impossible, it would have given seg fault, or not complained at all and returned 0. Its actually acessing kFmap[k][2] ,kFmap[k][3], kFmap[k][4]. and k | [0,2).

**elninio** · 09-21-2008

Originally Posted by laserlight

That's right, so my example is not correct.

In my defense, we're looking at code that has undescriptive variable names and magic numbers, with a complex mix of data structures that really should be simplified with the use of classes.

k only looped from zero to one is completely valid, because kFmap is only defined from 0 to 1. Whats the problem here?

Look at the data types.

**elninio** · 09-21-2008

Originally Posted by tabstop

Read your own code:

Code:

F.at(kFmap[k][namesmap[i]]) /= (10);

Let's suppose k is 0 and i is 4.
Then kFmap[0] is fine, that gets us a map. namesmap[4] is "four". We can then do the mapping -- kFmap[0]["four"] = 20. (You assigned it as kFmap[0][namesmap[4]], but that's fine.) So, the bit inside the at is 20 -- but F.at(20) throws an exception, since F only goes to 5.

You're absolutely right. Whats the point in using at() then? I want to divide some number that fits in int, by 10.

We can then do the mapping -- kFmap[0]["four"] = 20. 20/10 = 2. Thats what I want. My program is supposed to replace the '20' inside kFmap[0]["four"] with (20/10 = 2). So kFmap[0]["four"] should have 2 instead of 20.

**laserlight** · 09-21-2008

Originally Posted by elninio

Yes, thats the whole point. kFmap is only defined from 0 to 1.

Then you should write it in this way:

Code:

for (vector<vector<float> >::size_type k = 0; k < kF.size(); ++k) {

Then instead of:

Code:

for (int i = 1; i <= 4; i++){
    F.at(kFmap[k][namesmap[i]])++;
}

Write:

Code:

for (map<int, string>::iterator i = namesmap.begin(); i != namesmap.end(); ++i) {
    ++F.at(kFmap[k][i->second]);
}

Originally Posted by elninio

k only looped from zero to one is completely valid, because kFmap is only defined from 0 to 1. Whats the problem here?

It is hard to tell just what the 2 means.
If kFmap's size changed, you must change the magic number 2.
unsigned int is not quite the correct type to use (although it probably is).

**tabstop** · 09-21-2008

Do you want to replace the value in the mapping, or just use the value/10?

This leads me to think you want something like

Code:

F[k] = kFmap[k][namesmap[i]] / 10.0;

or perhaps

Code:

F[i] = kFmap[k][namesmap[i]] / 10.0;

or something like that.

**elninio** · 09-21-2008

Originally Posted by tabstop

Do you want to replace the value in the mapping, or just use the value/10?

This leads me to think you want something like

Code:

F[k] = kFmap[k][namesmap[i]] / 10.0;

or perhaps

Code:

F[i] = kFmap[k][namesmap[i]] / 10.0;

or something like that.

I want F[kFmap[k][namesmap[i]]] = ( F[kFmap[k][namesmap[i]]] / 10 );

Divide the current value by 10, and replace the value that was just divided.

Example:

If F[kFmap[k][namesmap[i]]] = 40. Then take 40, and replace it by 40/10, so before the loop we have 40, after the loop we have 4, at position k, i.

**laserlight** · 09-21-2008

I want F[kFmap[k][namesmap[i]]] = ( F[kFmap[k][namesmap[i]]] / 10 );

Divide the current value by 10, and replace the value that was just divided.

If that works, then just write:

Code:

F[kFmap[k][namesmap[i]]] /= 10.0;

**tabstop** · 09-21-2008

Originally Posted by elninio

I want F[kFmap[k][namesmap[i]]] = ( F[kFmap[k][namesmap[i]]] / 10 );

Divide the current value by 10, and replace the value that was just divided.

Example:

If F[kFmap[k][namesmap[i]]] = 40. Then take 40, and replace it by 40/10, so before the loop we have 40, after the loop we have 4, at position k, i.

But you can't do that -- F doesn't have a position (k, i), nor does it have a position 40. Is all you really want then

Code:

kFmap[k][namesmap[i]] /= 10;

to change the 40 where it is (inside kFmap)? I think you defined kFmap as a map from string to int, so you'll get integer division, and surely not a floating point.

**elninio** · 09-21-2008

Originally Posted by laserlight

If that works, then just write:

Code:

F[kFmap[k][namesmap[i]]] /= 10.0;

THATS WHAT I'VE BEEN DOING! BUT IT DOESN'T WORK!!!! AHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHH!!

That was the whole point of this thread.

**tabstop** · 09-21-2008

Originally Posted by elninio

THATS WHAT I'VE BEEN DOING! BUT IT DOESN'T WORK!!!! AHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHHH!!

That was the whole point of this thread.

You missed the very important word "If". What you said you wanted

I want F[kFmap[k][namesmap[i]]] = ( F[kFmap[k][namesmap[i]]] / 10 );

doesn't make any sense, since F[stuff] never existed in the first place, so you certainly can't divide it by ten.

Perhaps what you have written is in fact what you want, but if so you need to make sure that every element of kFmap is a valid index into F. Neither your original nor the example we've been working with has satisfied that requirement, which means you've been trying to divide <nonexistent memory> by 10, and failing at it.

**elninio** · 09-21-2008

Originally Posted by tabstop

You missed the very important word "If". What you said you wanted

doesn't make any sense, since F[stuff] never existed in the first place, so you certainly can't divide it by ten.

Perhaps what you have written is in fact what you want, but if so you need to make sure that every element of kFmap is a valid index into F. Neither your original nor the example we've been working with has satisfied that requirement, which means you've been trying to divide <nonexistent memory> by 10, and failing at it.

Your right - I'm investigating...

I guess its pure luck that its worked at all for single values?

**laserlight** · 09-21-2008

I suggest that your post your updated test program, along with the sample output. I really do suggest that you incorporate my suggested changes (though kF.size() should be kFmap.size()) - it pretty much eliminates two sets of possible out of bound errors that might occur.

Admittedly, I forgot one very important point: what exactly are you trying to do? You have to express that in words, not in code, since your code might not accurately express what you want.

**elninio** · 09-21-2008

Originally Posted by elninio

Your right - I'm investigating...

I guess its pure luck that its worked at all for single values?

I've tried making sure all the elements of kFmap[k][namesmap[i]] are within the resize value of F, but that didn't fix things.

**iMalc** · 09-22-2008

Ok we were mistaken earlier. But can you please write clearer code, and by that I mean common sub-expression elimination.
Something like this:

Code:

    		map<string,int> &myMap = kFmap[k];
    		for (int i = 1; i <= 4; i++){
    			int pos = myMap[namesmap[i]];
    			cout << pos << " kFmap | ";
    			cout << F.at(pos) << " | ";
    			F.at(pos) /= (10);
    			cout << F.at(pos) << " | ";
    			cout << endl;
    		}

You may find that it's more efficient too if the compiler was failing to do the approariate common sub-expression elimination, and if it's more readable - everyone wins.