If you start off with    %column = ( 2 => 1 ); then you'll just keep doing $column{2}= 1 over and over.

This seems like a case where you would want to use push on an array instead. (That is, unless you are planning on deleting things from the middle, in which case you don't want to use 1+number_of_keys as the next column number.) Or since you are only ever putting 1 in, you could simply have a scalar $max_column_number that you increment.

Even after reading 3 explanations from you about this code, I still don't see why you want to put data into a hash in this manner. How do you use this hash elsewhere?

...I can see what's going on here very clearly. The data is being stored in a hash using keys which are expected to be sequentially numbered integers. Or in simpler terms, we're taking a hash and making it perform as an array.

But, as others point out here, if the hash is initialized (or manipulated) in an unexpected way, the scheme breaks down.

Not to be glib, but...

I eat my peas with honey.
I've done it all my life.
It makes the peas taste funny.
But it keeps them on the knife.

Unless there's some rationale that is not clear from the original code snippet, I'd say use an array instead of a hash. Then push() will reduce the amount of logic required to get your information into the data structure.

...All the world looks like -well- all the world, when your hammer is Perl.
---v

To give the OP a few possible rationales:
1. My array would get so big, but is sparsely populated
2. My stuff isn't sequential (kinda doesn't work in this case, but is a good excuse, in general...)
3. I need to do a lot of random-access lookup
I give up. Essentially, imho, this is pointing out that the OP is trying to solve a problem without fully understanding the requirements necessary. If s/he did, then a less complex solution would probably point itself out very quickly.

------
We are the carpenters and bricklayers of the Information Age.

Don't go borrowing trouble. For programmers, this means Worry only about what you need to implement.

Please remember that I'm crufty and crochety. All opinions are purely mine and all code is untested, unless otherwise specified.

Ver nice response Limbic, my apologies for being unclear. I've replied to BrowserUK above and hope the explanation sheds some light on my problem.

The points you've made above are exactly what I've intended the code to do.

I have checked the results as you have suggested by using print, what seems to happen is that the result of $key_num seems to stay the same and it works for the last iteration of the loop, the last key number in %columns is always incremented by one, though it seems that no matter how many times I loop, the last inserted key is overwritten, seems to me (I used print) that $key_num always returns the same number.


BlackJudas

blackjudas,
The following code works fine for me - just as you want it to:

#!/usr/bin/perl -w
use strict;
my %columns = (
    1 => 1,
    2 => 1,
    3 => 1,
    4 => 1
);

my @discounts = qw( 1 2 1 3 1 4 1 5);

for my $discount (@discounts) {
    if ($discount == 1) {
        my $key_num = scalar keys %columns;
        $key_num++;
        $columns{$key_num} = 1;
    }
}
print "$_ : $columns{$_}\n" foreach (keys %columns);
[download]

Cheers - L~R

Update: As others have pointed out - if %columns is already populated and the keys are not sequential starting from one - you run the risk of this not working. This could also be a problem if you delete keys elsewhere. For instance:

17 => 1
3  => 1
[download]

There are 2 keys, add one you get 3, but guess what - 3 already exists. You may want to check to see if the key exists. I think there is a better way to accomplish whatever you are trying to do.

Then it is quite likely that you a missing one key value (as has been hinted at several places in this thread). What keys are in the hash when this code isn't working? If the keys are, for example, 1, 2, 3, 5, 6, and 7, then you have 6 keys and you will use "7" as your next key (since "4" is missing). Such would explain your problem.

This is part of why so many have complained about not understanding your code. Part of your code assumes that all keys from 1 through N will be present (and no others) which means an array would be a more appropriate data structure (and wouldn't be susceptible to this type of bug). And you have yet to explain how you use this hash elsewhere, so we are forced to guess why you'd choose what appears to be a very poorly fitting solution, so we can't help much with fixing it.

                - tye

$key_num always has the same value because you are always setting it to the same value. You are first setting it to the number of keys in %columns $key_nums = keys %columns;, and then you increment it by 1 $key_nums++;. You do this for every value in @discounts that has a value of 1?

It's completely unclear from your question what it is that you are actually trying to do....a written description of your requirement would probably clarify this.

The explanation of what I'm trying to do is going to be somewhat unclear, it is part of a database handler which parses output and creates HTML tables based on user selections. The %columns hash contains descriptors which (if valid) are parsed and a table is put together at a later time.

I have gone over the logic, the @discounts array contains a number of elements which have either a value of 0 or 1. If the current element has a value of 1, then count how many keys are in columns.



%columns = (
  1 => 1,
  2 => 1,
  3 => 1,
  4 => 1
);
[download]



In this case, I ask for how many keys are in %column, increment by one and move forward, so the above should return 4 += 1 = 5.

Then assign:

$columns{5} = 1;

Loop again, return a count of 5, increment and assign:



  $key_num = keys %columns # 5
  $key_num++; # 6
  $columns{6} = 1;
[download]



Is that a little more clear?

BlackJudas

Are you sure that @discounts contains a 1? If not, you'd never be assigning a new key.

Show us a tiny, complete script that demonstrates this problem.

Makeshifts last the longest.

This example is an over-simplified form of what the code actually looks like. @discounts contains a large number of elements that have a value of either 0 or 1.

If the element contains a 1 %columns is assigned a new key (name is based on how many keys actually exist in %columns) with a value of 1.

BlackJudas

------
We are the carpenters and bricklayers of the Information Age.

Don't go borrowing trouble. For programmers, this means Worry only about what you need to implement.

Please remember that I'm crufty and crochety. All opinions are purely mine and all code is untested, unless otherwise specified.