Program to show a number appears (N/2 + 1) times

Question

I'm a beginner at programming. I wrote a program to show that a number appears (N/2 + 1) times. But every time I run it, my cmd crashes. I am thinking maybe I have an error with my array.

My text file - values3.txt has the numbers {2, 3, 2, 2, 2, 3, 2, 1}

#include <stdio.h>

#define range_of_nums 3

int main(void)
{   
    int i;
    int num_of_values = 8;
    int a[num_of_values];
    int freq[range_of_nums];
    int current = 0;
    int found_number = 0;

    FILE *fp;

    fp = fopen("values3.txt", "r");

    for (i=0;i<num_of_values;i++)
    {
        fscanf(fp, "%d", &a[i]);
    }

    fclose(fp);

    for (i=0;i<num_of_values;i++)
    {
        current = a[i] - 1;
        freq[current] += 1;
    }

    num_of_values = num_of_values / 2;

    for(i=0;i<range_of_nums;i++)
    {
        if (freq[i] > num_of_values)
        {
            found_number = i+1;
        }
    }

    if (found_number != 0)
    {
        printf("The number %d occurs more than N/2 %d times \n", found_number, num_of_values);
    }

    else 
    {
        printf("No number occurs more than N/2 times\n");
    }

    return 0;
}

**Possible** (not a c guy) hint: https://stackoverflow.com/questions/1414215/initial-value-of-int-array-in-c (freq) — , Oct 15 '17 at 08:44
How exactly is the text file? Does it have the `{`, `,`, etc as well? — J...S, Oct 15 '17 at 08:54
"my cmd crashes", cmd.exe from windows crash or **your** program crash ? — Stargateur, Oct 15 '17 at 08:55
You also need to initialize the values in `freq` to 0. As it is, they could initially contain any value. — Tom Karzes, Oct 15 '17 at 09:01

score 1 · Answer 1 · answered Oct 15 '17 at 09:32

You are actually on the right path, you just fell off by failing to validate your file was open and in your loop logic at the end. You also have a problem in not validating that you actually got num_of_values values from your file.

Let's start by eliminating the "Where is the values3.txt file?" problem. main() provides the ability for your to pass arguments in, such as a filename, make use of that. The declaration for main accepting arguments is:

int main (int argc, char *argv[])  /* where argc arguments are passed in argv */

This eliminates the hardcoding of values3.txt in your program. Just pass the filename as the first argument and

FILE *fp = fopen (argv[1], "r");

Or, take it one better and use a ternary operator to open the file in argv[1] if it is given, otherwise read from stdin (by default):

FILE *fp = argc > 1 ? fopen (argv[1], "r") : stdin;

Always validate your file is open for reading, and handle the error if it is not, e.g.

if (!fp) {  /* validate file open for reading */
    fprintf (stderr, "error: file open failed '%s'.\n", argv[1]);
    return 1;
}

Another significant issue you had was failure to initialize freq to zero. When you start freq[current] += 1; in your code, freq[current] must have been set to zero beforehand to make that make any sense. A simple initialization is all you need:

    freq[range_of_nums] = {0},      /* you must initialize freq */

On to the logic issues. You want to read up to a maximum of num_of_values integer values into a, but no more. Reading even 1 more would invoke Undefined Behavior. There is no guarantee that there will always be 8 integers in the file. What if there were 6? What if there were 26? You must handle all cases. So instead of using a for (i = 0; i < num_of_values; i++) loop why not just read all the value and make sure you don't read more than num_of_values? Something like:

/* read each value up to num_of_values */
while (i < num_of_values && fscanf (fp, "%d", &a[i]) == 1)
    i++;

How could you adjust if you only read 6 values? Maybe:

if (i != num_of_values)             /* validate 8 values read */
    num_of_values = i;              /* if not adjust number   */

(note: you already know it cannot be more from your test in front of fscanf)

You don't necessarily want to alter your variable holding the number of elements in your array (that's kinda important to keep around). Why don't we use a new variable to hold N/2 (say num_by_2. Then you could do:

num_by_2 = num_of_values / 2;       /* you need another var */

while still preserving num_of_values for use later in your code.

Lastly, if I understand correctly, you want to identify all numbers that appear more than N/2 time. You have a logic problem in your loop layout. To find all values that appear greater than N/2 times, the printf needs to be in that loop, e.g.

for (i = 0; i < range_of_nums; i++)
{
    if (freq[i] > num_by_2)
    {
        found_number = i + 1;
        printf ("The number %d occurs more than N/2 %d times \n",
                found_number, num_by_2);
    }
}

Now you know if found_number has a value, a value was found. So your last test could simply be:

if (found_number == 0) 
    printf("No number occurs more than N/2 times\n");

Putting it altogether, you could clean the logic up with something similar to the following:

#include <stdio.h>

#define range_of_nums 3

int main (int argc, char **argv)
{   
    int i = 0,
        num_of_values = 8,              /* you have 8 values */
        a[num_of_values],               /* otherwise your VLA is wrong */
        freq[range_of_nums] = {0},      /* you must initialize freq */
        num_by_2 = 0,
        found_number = 0;
    FILE *fp = argc > 1 ? fopen (argv[1], "r") : stdin;

    if (!fp) {  /* validate file open for reading */
        fprintf (stderr, "error: file open failed '%s'.\n", argv[1]);
        return 1;
    }

    /* read each value up to num_of_values */
    while (i < num_of_values && fscanf (fp, "%d", &a[i]) == 1)
        i++;

    if (fp != stdin) fclose (fp);       /* close file if not stdin */

    if (i != num_of_values)             /* validate 8 values read */
        num_of_values = i;              /* if not adjust number   */

    for (i = 0; i < num_of_values; i++) /* set frequency array vals */
        freq[a[i] - 1] += 1;

    num_by_2 = num_of_values / 2;       /* you need another var */

    for (i = 0; i < range_of_nums; i++)
    {
        if (freq[i] > num_by_2)
        {
            found_number = i + 1;
            printf ("The number %d occurs more than N/2 %d times \n",
                    found_number, num_by_2);
        }
    }

    if (found_number == 0) 
        printf("No number occurs more than N/2 times\n");

    return 0;
}

Example Use/Output

$ echo "2 3 2 2 2 3 2 1" | ./bin/numby2freq
The number 2 occurs more than N/2 4 times

Look things over and let me know if you have further questions.

score 0 · Answer 2 · answered Oct 15 '17 at 09:02

0

This list of bugs can be non complete.

fp = fopen("values3.txt", "r");

You don't look if your file has been open

fp = fopen("values3.txt", "r");
if (fp == NULL) {
  perror("fopen()");
  return 1;
}

fscanf(fp, "%d", &a[i]);

You don't verify if fscanf sucess

int ret = fscanf(fp, "%d", &a[i]);
if (ret != 1) {
  fprintf(stderr, "error input");
  return 1;
}

current = a[i] - 1;
freq[current] += 1;

You trust user input, index could be out of range

current = a[i] - 1;
if (current < 0 || curent >= 8) {
  fprintf(stderr, "error range");
  return 1;
}
freq[current] += 1;

answered Oct 15 '17 at 09:02

Stargateur

24,473
8
65
91

I implemented your code and it says no such file or directory – Brady Oct 15 '17 at 09:11
@Brady So what is your conclusion ? – Stargateur Oct 15 '17 at 09:12

Program to show a number appears (N/2 + 1) times

2 Answers2