deprecate OptionInFile and OptionOutFile

[Peter]

Don't forget to add blurp in NEWS.

Replace classes in example in docs of CommandLine.

[Peter]

If deprecate these classes, I want to replace them with something.

One suggestion would be a class NewClass that can be used as follows:

CommandLine cmd;
OptionFile out(cmd, "o,out", "output to file", false, false, "w");
cmd.parse(argc, argv);
NewClass os(out.present(), out.value(), std::cout);
os << "hello world!\n"

in other words, the class inherits from an ostream and has a constructor:

NewClass(bool, const string&, ostream&)

and if bool is true it will write to an ofstream(string), and otherwise write to the ostream&. For symmetry reasons these constructors are also wanted:

NewClass(bool, ostream&, ostream&)
NewClass(bool, ostream&, const string&)
NewClass(bool, const string&, const string&) 

although the last one is perhaps a bit overkill.

[Jari Häkkinen]

Replying to peter:

in other words, the class inherits from an ostream and has a constructor:

NewClass(bool, const string&, ostream&)

and if bool is true it will write to an ofstream(string), and otherwise write to the ostream&.

Why bool in the constructor? Couldn't the ostream& simply either be ostream or an ofstream, i.e., the caller sets it appropriately?

[Peter]

Replying to jari:

Why bool in the constructor? Couldn't the ostream& simply either be ostream or an ofstream, i.e., the caller sets it appropriately?

How do you mean? Can you please be more specific and give an example.

[Jari Häkkinen]

Replying to peter:

Replying to jari:

Why bool in the constructor? Couldn't the ostream& simply either be ostream or an ofstream, i.e., the caller sets it appropriately?

How do you mean? Can you please be more specific and give an example.

If I understand your example correctly the "hello world!" string either ends up in a file specified by out.value() if out.present() returns true, otherwise the output is written to stdout.Modifying your example

CommandLine cmd;
OptionFile out(cmd, "o,out", "output to file", false, false, "w");
cmd.parse(argc, argv);
ostream& xxx=std::cout;
if (out.present()) {
    set xxx to a ostream associated with out.value();
}
NewClass os(xxx);
os << "hello world!\n"

Of course, the NewClass? interface changes completely. I presume there are ways to tie stdout with a selectable ostream, then the task is to make this connection within the if statement and no xxx variable would be needed ... and there wouldn't be a need for NewClass?. Hm, then the programmer would need to know how to tie streams with other. What is the rationale for NewClass?? Is it simply to provide means to write to selectable ostream?

[Peter]

Replying to jari:

If I understand your example correctly the "hello world!" string either ends up in a file specified by out.value() if out.present() returns true, otherwise the output is written to stdout.

You understand correctly, but I'm not sure I understand your code:

 CommandLine cmd;
 OptionFile out(cmd, "o,out", "output to file", false, false, "w");
 cmd.parse(argc, argv);

so far clear

 ostream& xxx=std::cout;

why is xxx needed? Any modification to xxx will also affect std::cout, so why not just stay with std::cout?

 if (out.present()) {
     set xxx to a ostream associated with out.value();
 }

1) How do you implement this set xxx to? 2) It looks like there is an ostream created (associated with out.value()), but won't that ostream go out of scope after the if block which implies xxx is invalid. You could of course create the ostream dynamically, but then my question is where the ostream is destructed.

 NewClass os(xxx);

xxx is either associated with std::cout or with a file "out.value()" so there is no need to create a new object NewClass; we can use xxx directly.

 os << "hello world!\n"

Of course, the NewClass? interface changes completely. I presume there are ways to tie stdout with a selectable ostream,

This sounds like ticket:520

then the task is to make this connection within the if statement and no xxx variable would be needed

exactly

... and there wouldn't be a need for NewClass?.

exactly

Hm, then the programmer would need to know how to tie streams with other.

Again, this sounds like ticket:520

What is the rationale for NewClass?? Is it simply to provide means to write to selectable ostream?

But ticket:520 is not sufficient here, because within the if statement we need to create an ofstream and the NewClass would collect the garbage of the created ofstream.

The straightforward way to implement the example code above without too much support would be

CommandLine cmd;
OptionFile out(cmd, "o,out", "output to file", false, false, "w");
cmd.parse(argc, argv);
ostream* xxx=&std::cout;
if (out.present()) {
    xxx = new std::ofstream(out.value().c_str());
}
*xxx << "hello world!\n"
if (out.present()) {
    delete xxx
}

but we wanna avoid repeating all this code over and over again, and that is the purpose of NewClass. Would you say NewClass is not needed?

[Peter]

Replying to peter:

Would you say NewClass is not needed?

Some people distinguish need and want; in that light no class in a library is strictly needed (as the user can implement it himself), so I rephrase my question: Would you say NewClass is not wanted?

[Jari Häkkinen]

Replying to peter:

why is xxx needed? Any modification to xxx will also affect std::cout, so why not just stay with std::cout?

Because xxx is a reference keeping track of where we want output to be written. xxx may change within the if statement

if (out.present()) {
    set xxx to a ostream associated with out.value();
}

and later use of xxx will automatically write (transparently) output to either cout or a user selected stream. Of course, in the if statement xxx is set appropriately, remember that xxx is a reference to a ostream so there must be a ostream associated with the out object (I am not implying that the out object should support anything with ostream but something needs to keep track of the extra ostream object). I was not trying to implement a complete solution, I wanted to start a discussion about the NewClass interface.

1) How do you implement this set xxx to? 2) It looks like there is an ostream created (associated with out.value()), but won't that ostream go out of scope after the if block which implies xxx is invalid. You could of course create the ostream dynamically, but then my question is where the ostream is destructed.

Yes, I did not touch these issues because it is not the main point of my argument. My main concern is the interface of NewClass.

 NewClass os(xxx);

xxx is either associated with std::cout or with a file "out.value()" so there is no need to create a new object NewClass; we can use xxx directly.

Yes, and this is the reason for my discussion in the last paragraph of http://dev.thep.lu.se/yat/ticket/521#comment:4

Of course, the NewClass? interface changes completely. I presume there are ways to tie stdout with a selectable ostream,

This sounds like ticket:520

What is the rationale for NewClass?? Is it simply to provide means to write to selectable ostream?

But ticket:520 is not sufficient here, because within the if statement we need to create an ofstream and the NewClass would collect the garbage of the created ofstream. [snip] but we wanna avoid repeating all this code over and over again, and that is the purpose of NewClass.

Yes, I agree that it is tedious to write the same code several times. The difference lies in that I'd prefer to have the caller to create the streams rather than the callee, and I do not understand the original proposal to send several streams to NewClass.

I suppose that the NewClass here is not the same as the NewClass of ticket:520?

[Peter]

Replying to jari:

Replying to peter:

why is xxx needed? Any modification to xxx will also affect std::cout, so why not just stay with std::cout?

Because xxx is a reference keeping track of where we want output to be written. xxx may change within the if statement

if (out.present()) {
    set xxx to a ostream associated with out.value();
}

and later use of xxx will automatically write (transparently) output to either cout or a user selected stream.

But you're missing my point. You cannot separate xxx from cout. Anything you do to xxx will also affect cout. Even &xxx are is equal to &cout. It would be another story if xxx was a pointer pointing at cout, because then you could reassign the pointer...

Yes, I did not touch these issues because it is not the main point of my argument. My main concern is the interface of NewClass.

As I read your comment you think there should be no NewClass. Is that right?

but we wanna avoid repeating all this code over and over again, and that is the purpose of NewClass.

Yes, I agree that it is tedious to write the same code several times. The difference lies in that I'd prefer to have the caller to create the streams rather than the callee,

If the caller creates the streams I don't see the need for this class. Then one can either use the solution in comment 5 or similarly using the new class in #521

if (out.present()) {
    ofstream* os = new ofstream(out.value().c_str());
    StreamRedirect rd = new StreamRedirect(cout, *os);
}
cout << "hello world!\n"
if (out.present()) {
    delete os;
    delete rd;
}

and I do not understand the original proposal to send several streams to NewClass.

OK, the constructor taking two streams are perhaps not so useful. Neither is the last one taking two strings. The second constructor (taking an ostream and a string) is likely the useful one that perhaps we should focus on.

I suppose that the NewClass here is not the same as the NewClass of ticket:520?

Yes they are different.

[Jari Häkkinen]

Replying to peter:

But you're missing my point. You cannot separate xxx from cout. Anything you do to xxx will also affect cout. Even &xxx are is equal to &cout. It would be another story if xxx was a pointer pointing at cout, because then you could reassign the pointer...

Ok

Yes, I did not touch these issues because it is not the main point of my argument. My main concern is the interface of NewClass.

As I read your comment you think there should be no NewClass. Is that right?

Well, as I see the NewClass here, it will be a wrapper to the class suggested in ticket:520

class NewClass(ostream& o, const string& s)
{
    ofstream ofs(s);
    my_NewClass520_ = NewClass520(o,ofs)
}

It will give the user a convenience class but I see little service from the class but an increase of the yat API.

If the caller creates the streams I don't see the need for this class. Then one can either use the solution in comment 5 or similarly using the new class in #521

if (out.present()) {
    ofstream* os = new ofstream(out.value().c_str());
    !StreamRedirect rd = new !StreamRedirect(cout, *os);
}
cout << "hello world!\n"
if (out.present()) {
    delete os;
    delete rd;
}

Isn't the code similar in your use case?

NewClass* nc = null;
if (out.present()) {
    nc = new NewClass(cout, out.value().c_str());
}
cout << "hello world!\n"
if (out.present()) {
    delete nc;
}

The question is really what classes should be included in yat, adding classes in ticket:520 and the one proposed in this ticket seems to me to be adding unnecessary complexity to the yat class library. Of course, not these specific classes but adding many of similar cases. As I write this comment I realize that these two classes are really one class with two different constructors?

NewClass(ostream, ostream);
NewClass(ostream, string);

[Peter]

Replying to jari:

It will give the user a convenience class but I see little service from the class but an increase of the yat API.

If that is how you see the class, I understand that you are sceptical. That class wouldn't add much.

Isn't the code similar in your use case?

No, if you check my initial suggestion the code would look like

NewClass nc(out.present(), out.value(), std::cout);
nc << "hello world!\n"

which is only two lines compared to 8 lines in your example. The class actually is an ostream and inherits from ostream. If any ofstream is created internally the class is taking care of destroying it.

I agree the class is a bit unconventional and I could possibly agree on letting it rest, but at least I want you to understand the idea before we shoot it down.

The question is really what classes should be included in yat, adding classes in ticket:520 and the one proposed in this ticket seems to me to be adding unnecessary complexity to the yat class library.

I'm not 100% convinced the class in ticket:520 is a good idea. As I understand, it only applies a std function in the constructor and then restores the classes involved in the destructor.

As I write this comment I realize that these two classes are really one class with two different constructors?

NewClass(ostream, ostream);
NewClass(ostream, string);

I disagree.

[Jari Häkkinen]

Replying to peter:

Isn't the code similar in your use case?

No, if you check my initial suggestion the code would look like

!NewClass nc(out.present(), out.value(), std::cout);
nc << "hello world!\n"

which is only two lines compared to 8 lines in your example. The class actually is an ostream and inherits from ostream. If any ofstream is created internally the class is taking care of destroying it.

Ah, now I understand the bool as the first parameter to NewClass, you move the if statement into the class. In that case the interface makes somewhat sense but I'd suggest another order of the parameters and a default on the bool

NewClass(ostream&, ostream&, bool=true)
NewClass(ostream&, const string&, bool=true)

This would include the class in ticket:520.

I agree the class is a bit unconventional and I could possibly agree on letting it rest, but at least I want you to understand the idea before we shoot it down.

I think I got it now and if you think there is an urgent need for the proposed functionality it should be added. What I react on is we may be creating a lot of different classes with closely related functionality. In this case, for me at least, it was hard to understand the difference between NewClass here and the class in ticket:520. I still think the two should be merged into one class. Isn't the difference between the two that in NewClass in this tickets takes car of the if statements (and in consequence the need to use pointers, new, and delete. At least for the caller.)?

As I write this comment I realize that these two classes are really one class with two different constructors?

!NewClass(ostream, ostream);
!NewClass(ostream, string);

I disagree.

Even with my suggested change to the interface?

[Peter]

Replying to jari:

Ah, now I understand the bool as the first parameter to NewClass, you move the if statement into the class. In that case the interface makes somewhat sense but I'd suggest another order of the parameters and a default on the bool

NewClass(ostream&, ostream&, bool=true)
NewClass(ostream&, const string&, bool=true)

This would include the class in ticket:520.

Hm interesting. I think your suggestion would do it. Though, it is different from what I intended. Let me explain the difference and that will also reveal if I've misunderstood you.

As I understand this class, JhClass, if we, for example, use it as follows

bool b;
...
JhClass jc(std::cout, "out.txt", b);
std::cout << "Hello World\n";

The output will go to cout if b is false and otherwise to a file out.txt. This is accomplished by redirecting the output of cout by letting cout use the buffer of std::ofstream("out.txt"). All of this is restored in the destructor as suggested in ticket #520.

My approach was a bit different. I wanted to create an object that was a std::ostream

class PjClass : public std::ostream

This class does not have its own buffer but is using the buffer of one of the streams in the constructor (or the private std::ofstream). Since the class is an ostream, you can use as an ostream

PjClass pc(std::cout, "out.txt", b);
pc << "Hello World!\n";

You can use std::cout in the same context and that output will always go to cout.

PjClass pc(std::cout, "out.txt", b);
pc << "Some output for cout or file 'out.txt'\n";
std::cout << "Some output for cout\n";

Is this an important feature? I'm not sure. The only obvious use case I can come up with is when debugging, and you would be surprised when your debug output doesnt show up on the screen. This could, of course, also be a feature, when you don't have access to any screen (running a cluster for instance).

I think I got it now and if you think there is an urgent need for the proposed functionality it should be added.

The reason is that I'm using OptionOutFile in quite many places and in particular the OptionOutFile::ostream() function. In some cases the option is is not declared mandatory which means that member function ::ostream() will return std::cout. When OptionOutFile will be deprecated I want it to be easy to update this kind of code.

What I react on is we may be creating a lot of different classes with closely related functionality. In this case, for me at least, it was hard to understand the difference between NewClass here and the class in ticket:520. I still think the two should be merged into one class. Isn't the difference between the two that in NewClass in this tickets takes car of the if statements (and in consequence the need to use pointers, new, and delete. At least for the caller.)?

We obviously need the same functionality for istream, and with your suggestion we can keep that within the same class (a few more constructors). With my suggestion it would need a separate class, so I think we should go for your suggestion.

So a couple of questions:

Should we support only char, i.e., ostream and istream, or should we support also other character types such as wchar_t. ostream is just a typedef as basic_ostream<char>, so one could implement this as an template on character type, but this could also be extended (without breaking anything) at a later point when/if we think it's needed.

Name of the class?

When we have agreed on a name I can commit my implementation on #520 and then we can extend that class.

[Peter]

Replying to peter:

but this could also be extended (without breaking anything) at a later point when/if we think it's needed.

This is not accurate. If we go from a non-templatized to a templatized implementation, we remove the class from the ABI, i.e., we remove it from libyat.*, which implies a breakage in compatibility. Sorry for the confusion.

[Peter]

added reminder about CommandLine docs in description

[Jari Häkkinen]

Replying to peter:

As I understand this class, JhClass, if we, for example, use it as follows

[snip]

Is this an important feature? I'm not sure. The only obvious use case I can come up with is when debugging, and you would be surprised when your debug output doesnt show up on the screen. This could, of course, also be a feature, when you don't have access to any screen (running a cluster for instance).

After your long explanation I see the difference in what I want to accomplish and what you try to do. Yes, the classes are different. What I like with JhClass? is that one can simply redirect all output to stderr or stdout by adding a short snip of code in the beginning of the main program. It does not provide the flexibility you are looking for.

I think the use case you provide may be confusing when looking at then output from a run. It is like looking at output to stderr and stdout where stdout is not flushed; stderr is usually written directly and messages appearing later in the source code may be displayed before stdout when stdout is not flushed. Until one starts to use flush, debugging may be awkward.

So a couple of questions:

Should we support only char, i.e., ostream and istream, or should we support also other character types such as wchar_t. ostream is just a typedef as basic_ostream<char>, so one could implement this as an template on character type, but this could also be extended (without breaking anything) at a later point when/if we think it's needed.

Why not go for future non-breaking?

Name of the class?

When we have agreed on a name I can commit my implementation on #520 and then we can extend that class.

I think your suggestion StreamRedirect? is good choice.

[Peter]

(In [2047]) closes #520 and refs #521. Implements the interface defined in #520 which should be extended to also include the interface discussed in #521

[Peter]

(In [2048]) adding bool parameter in constructors and fixed some docs typos

[Peter]

(In [2050]) refs #521. added constructor taking ostream and string. It does not take char* (as ofstream does) because I don't see the point with that. Needed to split member variable ios_ into a ostream* and istream* because the two cases need separate treatment (ostream needs to be flushed in destructor).

[Peter]

(In [2051]) the conclusion that the stream needed to flushed was nonsense, so I changed back to having and ios* as member variable. refs #521

[Peter]

(In [2052]) added constructor that creates and internal ifstream. refs #521.

[Peter]

(In [2053]) docs for StreamRedirector?. refs #521

[Peter]

(In [2054]) refs #521. Using OptionFile and StreamRedirector rather than deprecated classes

[Peter]

Trying to deprecate classes I experience some problem to get any response from gcc. It turned out that it is a ​bug in gcc that was fixed in 4.3. Older versions of gcc simply ignores __aytribute__((deprecated)). Here we want to deprecate classes OptionoutFile and OptionInFile, so the question is should we deprecate the classes, which implies it will not be recognized by older compilers. Or should deprecate the constructors of the class, which is recognized at least with gcc 4.0.1.

Thoughts?

[Jari Häkkinen]

I think we should deprecate classes but add code that trigger deprecation of constructors for gcc older than 4.3 using ​http://gcc.gnu.org/onlinedocs/gfortran/Preprocessing-and-conditional-compilation.html

[Peter]

(In [2057]) refs #521. Deprecate class OptionInFile? and OptionOutFile?. Replaced OptinOutFile? with base class OptionFile? in commandline_test

[Peter]

(In [2058]) deprecate constructors conditionally. Deprecation is active only for gcc and only for gcc older than 4.3. For 4.3 and newer the class deprecation should results in a warning. refs #521

[Peter]

(In [2059]) mention deprecation of OptionInFile? and OptionOutFile? in NEWS. closes #521