Write on file

[borgh]

HI,
I'm new with c++, I create a file with different name every x time, the first time the file is correct, other times is empty...
Where's the issue?

#include <chrono>
#include <fstream>
#include <string>

using namespace std;

int main()
{
    int i, l = 0;
    int iNum = 0;

    long microseconds;
    std::string name = "";

    name = "ciclocpp" + std::to_string(iNum) + ".txt";

    ofstream fout(name);

    for (int k = 0; k < 1000000; k++)
    {
        if (k > 0 &&  (k % 100000) == 0)
        {
            fout.close();
            iNum++;
            name = "ciclocpp" + std::to_string(iNum) + ".txt";
            ofstream fout(name);
        }

        auto start = std::chrono::high_resolution_clock::now();

        for (i = 0; i < 50000; i++)
        {
            l += i;
            if ((l % 4) == 0)
                l = l >> 1;
        }
        auto elapsed = std::chrono::high_resolution_clock::now() - start;

        microseconds = std::chrono::duration_cast<std::chrono::microseconds>(elapsed).count();


        fout << microseconds << "\n";
    }


    fout.close();
}

[salem c]

The fout on line 26 isn't the same as the fout on line 17.

Maybe
fout.open(name);

[Handy Andy]

Hello brogh,

you wrote:

I create a file with different name every x time,

So what value to you give to "x"?

The condition of the if statement on line 21 will never be true, so you are always writing to the same file.

You may want to take some time to understand how %, (mod), works. zero (0 % 1000000) will return the value of (0) zero and (1000000 % 1000000) will return (0) zero, but 1 - 999999 will return those numbers.

As I understand the code line 26 should be a different file name than line 17, but you never enter the if statement to create a different file name, so you are always writing to the same file.

When it comes to variable names try to avoid using a single letter name especially the lower case "L" and the capital "O". the "l" in some fonts can easily be mistaken as the number 1 and the "O" for the number zero.

If you are going to use "std::string" and "std::chrono" Then loose line 5 and write "std::ofstream". You will be doing your-self a favor.

Andy

[borgh]

Hi,
thanks to everybody, now it works

Andy, I want write 100k lines per file

Then loose line 5 and write "std::ofstream". You will be doing your-self a favor

What do you mean??

[Handy Andy]

Hello brogh,

I want write 100k lines per file

Then in line 21 change the 1,000,000 to 100,000. And the if statement will be true 10 times.

In the past there has been a lot said about using namespace std;. it would be better if you read this: http://www.cplusplus.com/forum/beginner/258335/ for a start. This is the most recent post on the subject. If you want more try a search on "using namespace std" with out the "".

While working on your program this is what I came up with. See what you think.

#include <iostream>
#include <chrono>
#include <fstream>
#include <string>
//#include <Windows.h>  // <--- Used for testing.

//using namespace std;

int main()
{
    constexpr int MAXLOOPS{ 101 }, MODNUM{ 50 };

    int /*i,*/ num{};
    int fileNum{};

    long microseconds{};
    std::string name;

    name = "ciclocpp" + std::to_string(fileNum) + ".txt";

    std::ofstream fout(name);

    for (int k = 0; k < MAXLOOPS; k++)
    {
        if (k > 0 && (k % MODNUM) == 0)
        {
            fout.close();

            fileNum++;

            name = "ciclocpp" + std::to_string(fileNum) + ".txt";

            //std::ofstream fout(name);  // <--- Trying to define a variable that already exists.
            fout.open(name);  // <--- Should be this. Switch comment to see the difference.
        }

        auto start = std::chrono::high_resolution_clock::now();

        for (int i = 0; i < 50000; i++)
        {
            num += i;
            if ((num % 4) == 0)
                num = num >> 1;
        }

        auto elapsed = std::chrono::high_resolution_clock::now() - start;

        microseconds = std::chrono::duration_cast<std::chrono::microseconds>(elapsed).count();

        //if (!fout)  // <--- Used for testing. Remove here to "else" for normal use.
        //{
        //    std::cerr << "\n     File problem!\n";
        //    std::cerr << "\n     Error " << GetLastError() << " ERROR_ALREADY_EXISTS, i.e., \"fout\" already defined.\n";
        //    return 1;
        //}
        //else
            fout << microseconds << "\n";
    }


    fout.close();
}

The numbers I used in line 11 are scaled down for testing. You will need to change those when the program is working.

Notice the difference between lines 33 and 34.

Lines 50 - 56 along with line 5 I used to figure out what was going wrong. I would suggest saving the file somewhere for future reference before deleting the commented parts.

Be advised that if you are not using Windows this may not work.

Andy

[borgh]

Hi Andy,
thank you so much for your answer
I'm unsing Windows 10
If I understand it's better not using namespaces std to have a clear code and don't make mess with names...
A question, you commentend int i at the begining and declared into for cycle, I made this way to declare i one time at the beginning, hoping to reduce memory access and so to be quicker...

[jonnin]

the std namespace is too big and has so many names you will collide with them if you pull the whole thing in unless you mangle all your own names and never use any common english words.

You can have both -- you can use smaller using statements for things like cin/cout etc and whatever you need in your current file, without pulling in the entire space. Or you can put std:: on everything, which is the popular style now.

for homework pulling in std is 'ok' but a 'bad habit'. If you do it on every homework you will forget and do it on real code when you get a job, which has a high chance of breaking something somewhere someday.

[jlb]

A question, you commentend int i at the begining and declared into for cycle, I made this way to declare i one time at the beginning, hoping to reduce memory access and so to be quicker...

In modern C/C++ it is considered a better practice to declare variables closer to first use instead of at one big glob at the beginning of some scope. Also you should strive to use meaningful variable and function names usually reserving single variable names for very limited scopes.


By the way since you only used i once, it won't reduce memory access, and let the compiler worry about "quicker", it's much better at that job.

[Handy Andy]

Hello brogh,

Sorry I was away from the computer for a couple of hours.

All I can do is add to what jlb said.

I have come to learn that there are some rare occasions when you might need the value of something like "i" outside the for loop when it ends. Otherwise it is better to define the loop iterator in the for loop where it becomes local to the for loop and is destroyed when the for loop ends.

@jlb,

In modern C/C++ it is considered a better practice to declare variables closer to first use instead of at one big glob at the beginning of some scope

Thank you. I never quite understood this until now.

Andy

[jonnin]

variable position can affect performance, and its hard to second guess it. Comes down to whether the compiler can ignore creating a variable and just borrow a register for a moment or not (for appropriate types of course, like loop counters), which is partly controlled by where you declare it, how you use it, and such. Declare all up front in a pile can confuse its ability to optimize them this way. Conversely declaring them too locally (like inside nested loops at some scope) can cause other problems, like creating and destroying an object (stl containers, your own stuff, etc) every loop iteration is significant overhead if the compiler can't figure out how to ignore it. Compilers are smart, but they have limits and err on the side of correctness over speed.
generally, I agree with the above -- declare near where it is used with the least scope it needs. I make an exception and try to not allocate anything inside loops or functions that are spam called in a loop. YMMV

[jlb]

I have come to learn that there are some rare occasions when you might need the value of something like "i" outside the for loop when it ends. Otherwise it is better to define the loop iterator in the for loop where it becomes local to the for loop and is destroyed when the for loop ends.

But if you do need the variable outside the loop then it should be named something meaningful.

[borgh]

Thanks for all your answers

About std namespaces it's clear, it's too big

About where to declare variables, as I told I declared all above hoping to have a quicker code execution, from what you say it's not this way, I'll consider this in my next code

About variable names I agree with you, usually I don't do so and I put the first char to get the type (iNum for int, lNum for long), this case I don't do couse this is just an exercize, I need to understand if is quicker a c++ code than a c# code. After some tests c++ seems quicker and execution time is more stable

Now, you told me that this code is just for Windows, what could I change to let it complile on Linux? Or where can I find some info?

[MikeyBoy]

Now, you told me that this code is just for Windows, what could I change to let it complile on Linux? Or where can I find some info?

I'm curious about this too. I don't see anything in the code that's Windows-specific. @Handy Andy can you clarify which bits of your code you believe won't work on Windows, and why?

[jlb]

About variable names I agree with you, usually I don't do so and I put the first char to get the type (iNum for int, lNum for long), this case I don't do couse this is just an exercize,

Including the "type information" in a variable name is considered a bad practice. Keeping your variables as local as possible with descriptive names is really all you should need.

Now, you told me that this code is just for Windows, what could I change to let it complile on Linux?

The only thing I see in Andy's post that is Windows specific is the #inclusion of "windows.h" and the call to GetLastError() which is not really needed.

[MikeyBoy]

The only thing I see in Andy's post that is Windows specific is the #inclusion of "windows.h" and the call to GetLastError() which is not really needed.

Both of which are already commented out, so I assumed that wasn't what he was talking about. The code as he's posted it should work on any platform.

[Handy Andy]

@Mikey Boy,

I was working on the belief that "Window.h" and the "GetLastError()" function are used with the Windows operating system and may not be available to other operating systems.

Since I do not have any other operating system to work with I do not have first hand knowledge of that can and can not be used.

If I am wrong in what I have said let me know. I am always open to learn something new and I can not correct what I do not know is wrong.

Andy

[borgh]

Including the "type information" in a variable name is considered a bad practice. Keeping your variables as local as possible with descriptive names is really all you should need.

@Jlb
Why is a bad practice?

So with "Window.h" and the "GetLastError()" commented it should compile on Linux?

[salem c]

https://en.cppreference.com/w/
If the header file isn't listed here, assume it is platform specific in some way.

[Ganado]

I feel like the discussion in this thread is pointlessly complicated. This is the beginner's section...

The original code has nothing platform-specific in it. Just forget this discussion about Windows happened.

[jlb]

Why is a bad practice?

At least two reasons, first what happens when you decide that the variable type is wrong? While in a small program it may not be too much of a problem to change every occurrence of the variable to a new name reflecting the type of variable. But in a large program it will be difficult to find every occurrence to change.

Second, IMO, adding the type information tends to obscure the code.

Also see this link for more information: https://isocpp.org/wiki/faq/style-and-techniques#hungarian