views:

743

answers:

4

Hello,

I'm C++ begginer. I did this excercise from Deitel's book:

Use a one-dimensional array to solve the following problem. Read in 20 numbers, each of which is between 10 and 100, inclusive. As each number is read, validate it and store it in the array only if it is not a duplicate of a number already read. After reading all the values, display only the unique values that the user entered. Provide for the "worst case" in which all 20 numbers are different. Use the smallest possible array to solve this problem.

Here is my code:

#include <iostream>

using namespace std;

bool compare(int arrayNum[],int arraySize,int element);

int main()
{
 const int size=20;
 int i=1;
 int num[size];
 int arrayElement;
 int counter=0;

 while(i<=20)
 {

  cin>>arrayElement;
  if(i==1)    //stores first element in array
   num[0]=arrayElement;
  //compare new element with stored elements
  //if element is not a duplicate store it in array
  else if (compare(num,size,arrayElement))
  {
   counter++;
   num[counter]=arrayElement;
  }


  i++;
 }
 //displays array elements
 for(int k=0;k<=counter;k++)
  cout<<num[k]<<endl;

 return 0;
}

//compare elements in array

bool compare(int arrayNum[],int arraySize,int element)
{
 for(int j=0;j<arraySize;j++)
 {
  if(arrayNum[j]==element)
   return false;
 }

 return true;
}

It works, but I'm not sure if I have interpreted the task correctly. I assume then I don't have to include conditional statement for range of numbers (10 to 100 inclusive), as this will be read from the keyboard and input by me. Therefore why was this instruction included? Also at the end it says

use the smallest possible array

I assume the max size has to be 20,but I don't think there is a way to dynamically expand array size (for example while storing new elements) as array size is const. I would appreciate if someone could help me with this. Any comments regarding code, or better solution are welcome.

+2  A: 

You do need a conditional. Basically it's saying to take in a number between 10 and 100. If it's not a number between those ranges, don't store it. Also, if that number already exists in the array, don't store it. Then just print out the values in the array.

You assume correct about the array size, it's maximum size would be 20, although you may not store all 20 values (again, because some input might be bad).

AlbertoPL
When it says "the smallest possible array" I interpreted it as the smallest size of the array, not the smallest number of values stored in the array. I think I made it more complicated than it was originally :). Anyway thanks for help.
Mike55
+2  A: 

The smallest possible array is a char[12], where the individual bits are used as flags.

char data[12]; // memset(data, 0, sizeof(data)); in main();

void set(int n_raw;)
{
   int n = n_raw - 10;
   int index = n/8;
   int offset = n-8*index;
   data[index] |= 1 << offset; // added "1 <<"
}

Reading the values back is left as an exercise for the student.

Steve Gilham
Would that need to be bitshifted instead? Offset is a number from 0 to 7 after pulling out the factor of 8 (which can be done with modulus too: n%8).data[index] |= (1 << offset);
Andrew
Yes. you're right. Will fix.
Steve Gilham
technically true, but you're no longer using the array as an array. If the intended solution had been to implement a bitset, I think the assignment text would have mentioned it. :)
jalf
Thanks Steve and Andrew,but honestly I don't understand it. Maybe I need to learn more , because I don't know what the "<<" does in "data[index] |= 1 << offset; statement. I know it can be used with cout, but I didn't know about array.
Mike55
@Pat: He's using the 12 * 8 = 96 bits as flags -- if he reads number 10, then he sets the first bit in the first char. if he reads number 100, then the second bit of the last char (90th bit in total) is set.Printing out the values will be then done by going through all the bits and writing which ones are set.
cube
Cute, but probably more optimized than the expected answer.
dmckee
yeah, rather than an array of 20+ values, it's a set of 90+ bit flags
jalf
<< is a bit-shift. To cut the long story short, different operators could be 'overloaded' to have different meanings. It's called operators overloading, and you would come across it.
Extrakun
Thanks again guys for the excellent explanation.
Mike55
+4  A: 

As each number is read, validate it and store it in the array

Emphasis mine. The text clearly says that your program has to validate the input. In other words, it has to check that the entered number is between 10 and 100, and if it is not, handle the error appropriately. So yes, you do need a conditional, although exactly what you do to handle the error is up to you.

And you're right, since arrays aren't dynamically resizable, the array size has to be at least 20.

jalf
I agree. Thanks Jalf.
Mike55
"what you do to handle the error is up to you". Anyway the question is slightly unclear what it means. I'd say that since it says "read in 20 values in the range", an out-of-range value should not count as one of the 20, just keep reading until you have seen 20 in-range values. But they might have meant that the numbers *will* all be in range, and by "validation" they mean "checking for duplicates". Or they might mean the program should halt when it sees invalid data, as a defensive programming measure. Lousy incomplete spec ;-)
Steve Jessop
True, the spec is unclear, and you could make a case for it simply meaning "validate that the number is not a duplicate". I think it's fair to assume that it should also be validated to fall within the 10-100 range. And yeah, the rule we were taught at uni was simple, "when the spec is incomplete, just make whatever assumptions you like, but document them". :)
jalf
I wish my customers would adopt that rule.
Steve Jessop
hehe yeah, but it makes sense for exercises and such in a teaching environment.
jalf
+1  A: 

I assume then I don't have to include conditional statement for range of numbers (10 to 100 inclusive), as this will be read from the keyboard and input by me. Therefore why was this instruction included?

Sure, in this case you wrote the code and know to enter numbers within the range expected; however, it is a standard best-practice to always validate input data from a user, always. That way erroneous values are handled gracefully (with an appropriate error message) and helps to make your program more robust.

Also at the end it says

use the smallest possible array

I assume the max size has to be 20,,,

I think what they might have been getting at here is that you could have used an array of 91 bytes as shown below.

int cnt = 0;      // count of unique values entered
byte values[91];  // values entered indicator array
int valIdx;       // used as values array index

memset((void *)values, 0, sizeof(values));  // initialize array to all zeros

while ( cnt < 20 )
{
   // [code for the following comments not shown]
   //   have user input a number
   //   validate the number (>= 10 && <= 100) 
   //   subtract 10 from the number and store it in valIdx 
   //   (now valIdx will contain a number >= 0 <= 90)

   if ( !values[valIdx] )
   {
      values[valIdx] = 1;
      ++cnt;
   }
}

// then to show the unique values...

for ( valIdx = 0; valIdx < sizeof(values); valIdx++ )
{
   if ( values[valIdx] )
   {
      cout << valIdx + 10 << endl;
   }
}

That solution however, would not have met the "use the smallest array possible" requirement.

Back to your code...

I would go ahead and add the user input validation, just for completeness (and to get into the habit of never trusting users (especially yourself).

As far as improvements go, here is one thing to think about. The compare routine goes through every array element when a unique number has been entered. It only needs to check against those that have a stored value in them. Making that change should lead you to refactor the contents of your while loop as well.

Robert
Thanks. I will fix it.
Mike55