0

I have come across the following problem.

I have a program which allows the user to create a .txt file and add up to 10 ASCII values. I am then closing the file, and re-opening in read mode. The point of this being that I am converting the inputted ASCII to integer using ATOI.

The working code for this is provided below.

My problem is: I want to create some sort of array which stores these inputted ASCII values. BY doing so, this will enable me to call upon a function to check which of these ASCII values is smallest.

fp = fopen("c:\\CTEMP\\1.txt", "w+");
{
    for (x = 0; x < 10; x++)
    {
        printf("\nType the word you want to add. Type exit to terminate: ");
        scanf("%s", word); // word is declared as char word[10]

        if (strcmp(word, "exit") == 0)
        {
            break;
        }
        fprintf(fp, "%s\n", word);
        words++;
    }
    fclose(fp);
}
fp = fopen("C:\\CTEMP\\1.txt", "r");

while (!feof(fp))
{
    fscanf(fp, "%s", &word);
    number = atoi(word);
    printf("\nstring  is \t %s\n", word);
    printf("integer is \t %d\n", number);

    //  location = find_minimum(array,number);
    //  minimum = array[location];

    //  printf("Minimum element location = %d and value = %d.\n", location + 1, minimum);       
}
scanf_s("%d");
}
  1. Am I tackling the problem of finding the smallest ASCII value correctly?
  2. Is there any other way without creating another array to store the ASCII values?
rrz0
  • 2,182
  • 5
  • 30
  • 65
  • 1
    [why `while(!feof(fp))` is wrong](http://stackoverflow.com/questions/5431941/while-feof-file-is-always-wrong) – Barmar Jan 24 '17 at 21:29
  • 1
    You don't need an array. Use a variable to hold the current smallest value. After you convert the string to a number, check if it's smaller than the variable. If it is, put the new value into the variable. – Barmar Jan 24 '17 at 21:31
  • You can do this in the loop that's reading from the file, you don't need to put them into an array first, unless you have other reasons to need all the inputs. – Barmar Jan 24 '17 at 21:32
  • 1
    I have read that `while!feof(fp))` is wrong because (in the absence of a read error) it enters the loop one more time than the author expects. Thanks for pointing this out, I will alter in my future codes. – rrz0 Jan 24 '17 at 21:34
  • You should post a http://stackoverflow.com/help/mcve. If you're just looking for a minimum of some sort you only need to store one element (the potential minimum candidate). No need to store the rest. – Petr Skocik Jan 24 '17 at 22:13
  • Hi, I understand your point, however I am looking to implement a function for this task and therefore did not think I could have shortened the code any less. I will keep this in mind. – rrz0 Jan 24 '17 at 22:15

2 Answers2

1

As mentioned by Barmar, there is no need to store all the values in an array just for the sake of finding the minimum then. Let a variable minNr store the smallest number read so far, and let minIdx store it's index. Whenever the number read in the current pass is smaller (or equal) then minNr, adapt minNr and minIdx accordingly. Thereby, for any two equal numbers read in, the latter will be considered as the minimum's index. Note that minNr is initialized with INT_MAX, such that already the very first number read in will "beat" this initial value:

int finished = 0;
int minNr = INT_MAX;
int minIdx = 0;

fp = fopen("C:\\CTEMP\\1.txt", "r");
if (fp==NULL)
  finished=1;

for (int i=1; !finished; i++)
{
    char word[50];
    if (fscanf(fp, "%s", word) < 1)
        finished = 1;
    else {
        int number = atoi(word);
        printf("\nstring  is \t %s\n", word);
        printf("integer is \t %d\n", number);

        if (number <= minNr) {
            minNr = number;
            minIdx = i;
        }
    }
}
if (minIdx > 0)
    printf ("min number is %d at position %d\n", minNr, minIdx);
else
    printf("no numbers read in; hence: no minimum calculated.");

BTW: in your code, if word is declared as something like char word[50], then statement fscanf(fp, "%s", &word) should give you at least a compiler warning because of the superfluous &.

Stephan Lechner
  • 34,891
  • 4
  • 35
  • 58
  • Thanks @Stephan for your insight and thoughts. It is unclear what you mean by "`minNr is initialized with INT_MAX, such that already the very first number read in will "beat" this initial value`" Some feedback on this would be greatly appreciated. – rrz0 Jan 24 '17 at 22:47
  • 1
    Consider that you init `minNr` with, let's say, `100`. If the file contained only values `> 100`, then the minimum of the file's numbers would not be detected but would remain `100`. So, `minNr` should be set to the largest possible number, which is `INT_MAX` OK? – Stephan Lechner Jan 24 '17 at 23:04
  • 1
    Thanks, understood. Does this code render `while (!feof(fp))` unnecessary or does it still have to be included after the `fopen` statement? – rrz0 Jan 24 '17 at 23:08
  • It's actually unnecessary, as it is not capable of detecting an empty file unless one performs at least one read operation before calling `feof(fp)`. Additionally, it would crash if the file could not be opened, i.e. if `fp==NULL`. Hence, `while(!feor(fp))` has a good chance to do something different than that what you want. I adapted the answer accordingly in order to prevent the code from crashing. – Stephan Lechner Jan 25 '17 at 00:06
1

My problem is: I want to create some sort of array which stores these inputted ASCII values. BY doing so, this will enable me to call upon a function to check which of these ASCII values is smallest.

I don't know what an ASCII value is but here's how you:

 char chars[10]; //create an array of 10 characters
 int ints[10]; //create an array of 10 ints
 char strings[10][10]; //create an array of 10 9-character strings 

Am I tackling the problem of finding the smallest ASCII value correctly?

Semantically, perhaps. But you're 1) being spacially inefficient by trying to store all the elements (you probably don't need to store them -- not in memory, not on disk) and 2) you're failing to do error handling on the IO functions you call. Additionally 3) you're creating a security hole with scanf("%s", without a length limit.

Is there any other way without creating another array to store the ASCII values?

Yes. Store only your current candidate for the minimum value. The rest can be forgotten.

Petr Skocik
  • 58,047
  • 6
  • 95
  • 142