Skip to main content
noticed another problem and pointed it out
Source Link
ArrayBolt3
  • 245
  • 1
  • 9

Worse yet is a fumble with randLte(). The name stands for "RANDom Less Than or Equal". Not only that, but a comment specifies that it generates values less than or equal to a provided upperBound argument. Except it doesn't. It produces values up to but excluding upperBound. So it should be renamed randLt, the comment changed to reflect what it really does, and the programmer should pay more attention to how the modulo operator actually works. (Hilariously, despite the fact that I misunderstood what my function did, what it actually did was what I needed it to do, which was also different than what I thought I needed it to do. Thus why you shouldn't code while tired, I guess.)

One way to fix this might be to use an array of 16 numbers representing the indexes that still can be selected, rather than using the array to represent the indexes that have already been selected. When one is selected, move a number from the end of the array into the position where the old one was. The i counter can be used to keep track of how many usable items are left in the array, and the unused portion can be ignored. So something like this might work (this is untestedfixed and tested):

int remainingIdxs[16]remainingDice[16] = { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15 };

for (i = 15;i16;i >= 0;i1;i--) {
    idx = randLte(i);
    letter = randLte(6);
    board[i]board[i - 1] = dice[remainingIdxs[idx]][letter];
    remainingIdxs[idx] = remainingIdxs[i];remainingIdxs[i - 1];
}

One way to fix this might be to use an array of 16 numbers representing the indexes that still can be selected, rather than using the array to represent the indexes that have already been selected. When one is selected, move a number from the end of the array into the position where the old one was. The i counter can be used to keep track of how many usable items are left in the array, and the unused portion can be ignored. So something like this might work (this is untested):

int remainingIdxs[16] = { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15 };

for (i = 15;i >= 0;i--) {
    idx = randLte(i);
    letter = randLte(6);
    board[i] = dice[remainingIdxs[idx]][letter];
    remainingIdxs[idx] = remainingIdxs[i];
}

Worse yet is a fumble with randLte(). The name stands for "RANDom Less Than or Equal". Not only that, but a comment specifies that it generates values less than or equal to a provided upperBound argument. Except it doesn't. It produces values up to but excluding upperBound. So it should be renamed randLt, the comment changed to reflect what it really does, and the programmer should pay more attention to how the modulo operator actually works. (Hilariously, despite the fact that I misunderstood what my function did, what it actually did was what I needed it to do, which was also different than what I thought I needed it to do. Thus why you shouldn't code while tired, I guess.)

One way to fix this might be to use an array of 16 numbers representing the indexes that still can be selected, rather than using the array to represent the indexes that have already been selected. When one is selected, move a number from the end of the array into the position where the old one was. The i counter can be used to keep track of how many usable items are left in the array, and the unused portion can be ignored. So something like this might work (fixed and tested)

int remainingDice[16] = { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15 };

for (i = 16;i >= 1;i--) {
    idx = randLte(i);
    letter = randLte(6);
    board[i - 1] = dice[remainingIdxs[idx]][letter];
    remainingIdxs[idx] = remainingIdxs[i - 1];
}
Remove an incomplete sentence chunk
Source Link
ArrayBolt3
  • 245
  • 1
  • 9

C is an easy language to make this sort of mistake in, but surely there are ways around it. One way I could try to keep a running list of what variables need to be freed before my job is done, perhaps by making a comment within or nearby the dictionary() function.

C is an easy language to make this sort of mistake in, but surely there are ways around it. One way I could try to keep a running list of what variables need to be freed before my job is done, perhaps by making a comment within or nearby the dictionary() function.

C is an easy language to make this sort of mistake in, but surely there are ways around it. I could try to keep a running list of what variables need to be freed before my job is done, perhaps by making a comment within or nearby the dictionary() function.

corrected a misunderstanding of mine in regard to C89 variable declaration
Source Link
ArrayBolt3
  • 245
  • 1
  • 9

Not so in Open Watcom C. While other compilers probablycan handle willy-nilly declaration just fine, Open Watcom insistsadheres to the C89 standard in this regard, and requires that all thevariable declarations beoccur at the topbeginning of a scope. I handled this by putting them all at the start of each function. This means that when reading a function, the first thing you get confronted with are all the somewhat-out-of-context variable names. Names like usedIdxs, idx, and letter are all annoying for this reason. Perhaps usedDice, selectedDie, and selectedDieLetter would be better. Moving the declarations to more sensible locations would also help.

Not so in Open Watcom C. While other compilers probably handle willy-nilly declaration just fine, Open Watcom insists that all the declarations be at the top of a function. This means that when reading a function, the first thing you get confronted with are all the somewhat-out-of-context variable names. Names like usedIdxs, idx, and letter are all annoying for this reason. Perhaps usedDice, selectedDie, and selectedDieLetter would be better.

Not so in Open Watcom C. While other compilers can handle willy-nilly declaration just fine, Open Watcom adheres to the C89 standard in this regard, and requires that all variable declarations occur at the beginning of a scope. I handled this by putting them all at the start of each function. This means that when reading a function, the first thing you get confronted with are all the somewhat-out-of-context variable names. Names like usedIdxs, idx, and letter are all annoying for this reason. Perhaps usedDice, selectedDie, and selectedDieLetter would be better. Moving the declarations to more sensible locations would also help.

Source Link
ArrayBolt3
  • 245
  • 1
  • 9
Loading