0

I am trying to read the data in from a text file, and then divide each value by 3, before printing the output value for each new value.

An example of the format of the text file can be seen below:

0.00707946 -0.0241935 23.9401 0 0.307334 0.2046

As can be seen from the above example, every value is seperated by a space, the number of significant figures vary, and numbers can be positive or negative. I can successfully read through and print out the values to cmd as characters, however, my aim is to divide every single value by the number 3 (an integer) which I am struggling to do.

Is my issue the choice of format specifier I have chosen in my printf statement? Or the choice of explicit casting to float (which I chose to do as some of the numbers are floating point values)

What I have tried so far:

   #define _CRT_SECURE_NO_WARNINGS

#include <stdio.h>
#include <stdlib.h>

int main()
{
    char file_name[25];
    int current_value = 0; 
    int new_value; 


    FILE *fp;   //file handle 

    printf("Enter filename: \n");
    gets(file_name);

    fp = fopen(file_name, "r"); // read mode

    //error handling
    if (fp == NULL)
    {
        perror("Error while opening the file.\n");
        getchar(); 
        exit(EXIT_FAILURE);
    }

    while (fscanf(fp, "%d", &current_value) != EOF)     //while end of file has not been detected 
    {
        new_value = current_value / 3; 
        printf("%d ", new_value);

    }

    fclose(fp);
    getchar(); 
    return 0;
}
p.luck
  • 646
  • 2
  • 9
  • 34
  • 2
    You will be interested in [Why gets() is so dangerous it should never be used!](https://stackoverflow.com/questions/1694036/why-is-the-gets-function-dangerous-why-should-it-not-be-used). You are trying to diving each *character* by 3. That is not what you need (and will likely result in generating non-printable characters). Is that what you really want? Or, do you want to reach each floating-point value and divide that by 3? – David C. Rankin May 05 '19 at 20:31
  • @DavidC.Rankin want to reach each floating-point value and then divide that by 3. – p.luck May 05 '19 at 20:32
  • 2
    OK, that makes much more sense. You will want to look at `fscanf` (or [fscanf_s](https://learn.microsoft.com/en-us/cpp/c-runtime-library/reference/fscanf-s-fscanf-s-l-fwscanf-s-fwscanf-s-l)). You can then read (and validate) a floating point value at a time and then divide. – David C. Rankin May 05 '19 at 20:33
  • Ok, so would I have to change the while loop conditions to: `while (fscanf(fp, "%d", &current_value) != EOF)`? – p.luck May 05 '19 at 20:41
  • 1
    `%d` is for integers, use `%lf` for `double`s or `%f` for `float`. You have to change the type of `new_value` accordingly of course. *`fscanf(/* ... */) != EOF`* is often seen. Please note that the comparison with `EOF` is only reliable when you try to read only one value at once because `fscanf()` returns the number of parameters successfully assigned but only returns `EOF` when reading fails before assigning at least one parameter. So `fscanf()` could return 1 if it can extract and assign one value but still fail at 2nd 3rd and not return `EOF`. – Swordfish May 05 '19 at 20:46
  • 1
    You also have to decide how to print out the new values. For example, if the input is 1, what should the result be printed as? If you divide by 3 (an int) should it use integer division (1/3 = 0), or double division to get one third? And how to print one third: 0.3, or .3333333333333333, or what? – FredK May 05 '19 at 20:56

1 Answers1

3

First, Never, never, Ever use gets(), see Why gets() is so dangerous it should never be used!. With that out of the way, you are attempting to divide each character by 3, not each floating-point value. atoi is an integer conversion for strings, not individual characters.

But all that aside, your at least provided a good-faith attempt at a solution. So let's look at how you can improve things. First, don't use magic-numbers, 25 in your code is a magic-number, instead if you need an integer constant, #define one, e.g.

#define _CRT_SECURE_NO_WARNINGS   //preprocessor requirement

#include <stdio.h>
#include <stdlib.h>
#include <string.h>

#define FNMAX 512   /* if you need a constant, define one (or more) */

int main (void) {

Also, Don't skimp on buffer size! On Linux the default PATH_MAX constant is 4096. 25 doesn't even begin to cover allowable filenames.

Next, replace gets with fgets. The only caveat is that you now must trim the trailing '\n' from the buffer. You can do that simply with strcspn (which will report the number of characters that do not include those in the reject string). So choosing a reject string of "\r\n" covers you and strcspn returns the number of character up to the first of either. You simply nul-terminate your string at that index overwriting the line-ending, e.g.

    printf ("Enter filename: ");
    if (!fgets (file_name, FNMAX, stdin)) {     /* validate EVERY input */
        fputs ("(user canceled input)\n", stderr);
        return 1;
    }

    file_name[strcspn(file_name, "\r\n")] = 0;  /* trim '\n' from end */

Good job on validating your file was open for reading before using fp. Now you simply need to continue in a manner that will read floating-point numbers instead of characters. While I would generally recommend reading the remaining lines into a buffer and then calling sscanf to parse the values from it, you can also just use fscanf to read the floating-point numbers one-after-the-other. (all scanf conversions except "%c" and "%[...]" discard leading whitespace)

You can very simply use fscanf to read, and then divide by 3 (which is where I violate the magic-number rule :), e.g.

    /* read/print each floating-point value and value divided by 3 */
    while (fscanf (fp, "%lf", &value) == 1)
        printf ("\nvalue: %.4f\ndiv-3: %.4f\n", value, value / 3);

That's it. Putting it altogether, you could do:

#define _CRT_SECURE_NO_WARNINGS   //preprocessor requirement

#include <stdio.h>
#include <stdlib.h>
#include <string.h>

#define FNMAX 512   /* if you need a constant, define one (or more) */

int main (void) {

    char file_name[FNMAX];
    double value;
    FILE *fp;   //file handle 

    printf ("Enter filename: ");
    if (!fgets (file_name, FNMAX, stdin)) {     /* validate EVERY input */
        fputs ("(user canceled input)\n", stderr);
        return 1;
    }

    file_name[strcspn(file_name, "\r\n")] = 0;  /* trim '\n' from end */
    /* open/validate file open for reading */
    if ((fp = fopen (file_name, "r")) == NULL) {
        perror ("fopen-file");
        return 1;
    }
    /* read/print each floating-point value and value divided by 3 */
    while (fscanf (fp, "%lf", &value) == 1)
        printf ("\nvalue: %.4f\ndiv-3: %.4f\n", value, value / 3);

    fclose(fp); /* close file */

    getchar();  /* hold terminal open on windows */

    return 0;
}

Example Use/Output

$ ./bin/readdivfloats
Enter filename: dat/floats.txt

value: 0.0071
div-3: 0.0024

value: -0.0242
div-3: -0.0081

value: 23.9401
div-3: 7.9800

value: 0.0000
div-3: 0.0000

value: 0.3073
div-3: 0.1024

value: 0.2046
div-3: 0.0682

Compiling From the Command Line

If the reason you have getchar() at the end of your code is to hold the terminal window open after your program finishes due to your using the Visual Studio IDE, you may want to consider just using the command line for small projects. (1) you don't have to set up a project in VS, (2) you can compile many different source files from the same directory in the time it takes to setup another project, and (3) you learn what compiler options you need, so you can then tell the IDE how you want your code compiled.

If using VS, it provides the "VS Developers Command Prompt", which is just a cmd.exe (Command Prompt) with the appropriate path and compiler environment variables set. The VS compiler is cl.exe. All you need to do to compile this code (in filename readdivfloats.c is:

cl /nologo /W3 /wd4996 /Ox /Fereaddivfloats readdivfloats.c

The /Fe option just names the resulting executable, so here it will be readdivfloats.exe in the same directory. I generally like to keep my source directory clean, so I create obj and bin subdirectories to put all the object files and executables in. The /Fo option let's you name the object file (or you can simply name a directory and the object files will be named with the name of the source file). So with that in mind, to put the object file below the .\obj subdirectory and the exe in the .\bin subdirectory, you would use:

cl /nologo /W3 /wd4996 /Ox /Foobj/ /Febin/readdivfloats readdivfloats.c

/W3 turns on full warnings, /wd4996 disables warning 4996, (the annoying #define _CRT_SECURE_NO_WARNINGS warning), Ox turns on fast optimization. You can see all options simply by entering cl /? in the terminal.

David C. Rankin
  • 81,885
  • 6
  • 58
  • 85
  • C. Ranking I really appreciate you providing me with such a detailed explanation, so thank you very much. The link to the question regarding why gets() is dangerous was a useful read, too- I will be sure to avoid using it in the future! I also was unaware about using magic numbers, that's a very interesting point and I will be sure to not use those. – p.luck May 05 '19 at 22:39
  • Glad it helped. There is no better language to learn than C. Nothing provides the same low-level control and raw speed. There is a lot to learn and don't think of it as a race. Best advice I can give is simply to *slow-down*, enjoy the learning process. "Learning-C", isn't something you ever quite complete, there is that much of it, and they tend to tweak the standard every few years or so. Good luck with your coding (and learning)! – David C. Rankin May 05 '19 at 22:44
  • I completely agree. I was initially taught Java and Python. Only this year have I started learning C, and its efficiency and good performance make it by far my favourite language. Thanks again! – p.luck May 05 '19 at 22:49