According to Alexis Mikhailov:
> Gilles Detillieux wrote:
> > > int
> > > String::readLine(FILE *in)
> > > {
> > > Length = 0;
> > > allocate_fix_space(2048);
> > >
> > > while (fgets(Data + Length, Allocated - Length, in))
> > > {
> > > Length += strlen(Data + Length);
> > > if (Length == 0)
> > > continue;
> > > if (Data[Length - 1] == '\n')
> > > {
> > > //
> > > // A full line has been read. Return it.
> > > //
> > > chop('\n');
> > > return 1;
> >
> > I think instead of the chop() and return above, you just want a "break;",
> > to be consistent with the >> operator below.
>
> If there will be 'break' then line consiting of only '\n' will return 0
> to calling function. I tried to make this function as close to
> ExternalParser::readLine as possible to remove readLine from
> ExternalParser.
You're right. I guess I wasn't thinking clearly when I wrote this. This
function looks fine just as it is.
>
> > > }
> > > if (Allocated > Length + 1)
> > > {
> > > //
> > > // Not all available space filled. Probably EOF?
> > > //
> > >
> > > continue;
> > > }
> > > //
> > > // Only a partial line was read. Increase available space in
> > > // string and read some more.
> > > //
> > >
> > > reallocate_space(Allocated << 1);
> > > }
> > > chop('\n');
> >
> > I guess the chop() is a judgement call - some may prefer to leave the '\n'
> > in the string, while some would prefer it taken out. I guess for our
> > purposes, we always want it out, and it's easy to add it if we want.
>
> See above for my reason to remove '\n'.
Yes, I guess consistency with ExternalParser::readLine is a good goal.
However, I think it would be more important to have consistency with
the >> operator we define, so that we have functionally equivalent
means of inputting lines from FILE or istream. I'd say that should be
our determining factor.
I noticed that the C++ library does have a >> operator for (char*).
I don't know the semantics of it, but if it reads a line, we should make
the String >> operator work the same way, leaving the \n in if the char*
>> operator leaves it in, and stripping it if the char* >> operator does.
Then, readLine should also be consistent with that, and anything that
uses these new methods should be written to work with that, stripping
the newline if needed.
Having said that, I've also noticed in the htlib/Configuration.cc code
that when reading the configuration, it strips both \r and \n. Not a bad
idea, given that files may get edited by non-Unix editors. It might make
sense in ExternalParser.cc to also strip out \r, in cases where external
parsers spit out lines ending in \r\n, but this is a minor point right now.
> > >
> > > return Length > 0;
> >
> > Should an empty line return 1 or 0? This code will return 0 on any empty
> > line, not just at EOF.
>
> No it will not. Because of return 1 above on empty line will be returned
> 1.
Yes, of course.
> > > }
> > >
> > > istream &
> > > operator >> (istream &in, String &line)
> > > {
> > > line.Length = 0;
> > > line.allocate_fix_space(2048);
> > >
> > > while (in.get(line.Data + line.Length, line.Allocated - line.Length))
> > > {
> > > line.Length += strlen(line.Data + line.Length);
> > > int c = in.get();
> >
> > I'm not clear on the semantics of ifstream::get() above, and why you need
> > two calls to it. Wouldn't it make more sense to use in.getline() instead,
> > and keep the logic of this operator as close to the readLine() method
> > above as possible? I believe ifstream::getline()'s semantics are identical
>
> IIRC getline will remove '\n' from buffer as well as from input stream.
> So I won't be able to find out whether '\n' was read or just maximum
> number of bytes read. istream::get(char *, int maxlen) reads input
> stream
> until '\n' or maxbytes read leaving '\n' in input stream.
OK. I got the impression from the Configuration code that getline didn't
strip the \n, because Configuration::Read() had a line.chop("\r\n"); after
calling getline() and appending the buffer to the line String. Of course,
that doesn't mean that getline() hasn't already stripped out the \n, does
it? So, it makes sense to use get() instead of getline(). Also, if
istream::get(char *, int maxlen) leaves the \n in the input stream, it
also makes perfect sense to do another get() to fetch that newline.
> > to fgets(), except that is works on an ifstream instead of a FILE. BTW,
> > I think it's ifstream, and not istream.
>
> IIRC ifstream is a subclass of istream. (I don't have GNU implementation
> at hand, but it is so in Borland).
I'm still quite new to C++, so I'm still learning. I noticed that
Configuration::Read() used ifstream, so I thought that's what the input
class was called. Of course, it's a subclass, so all the istream methods
and operators will apply to ifstream as well.
> > > if (c == '\n')
> > > {
> > > //
> > > // A full line has been read. Return it.
> > > //
> > > break;
> > > }
> > > if (line.Allocated > line.Length + 2)
> > > {
> > > //
> > > // Not all available space filled. Probably EOF?
> > > //
> > >
> >
> > Will this code add EOF to the Data?
>
> Oops. It can. In case if there is a 2048 bytes in input stream
> exactly (or 4096 or ...). In other cases 'while' will exit by
> condition. So condition above instead of 'if (c == '\n')'
> should be 'if (c == '\n' || c == EOF)'.
OK, I'll take another look at the full code you posted last week for
this >> operator, keeping this one change in mind, as well as my new
understanding of the get() method(s), and I'll see if it makes sense
to me now.
--
Gilles R. Detillieux E-mail: <[EMAIL PROTECTED]>
Spinal Cord Research Centre WWW: http://www.scrc.umanitoba.ca/~grdetil
Dept. Physiology, U. of Manitoba Phone: (204)789-3766
Winnipeg, MB R3E 3J7 (Canada) Fax: (204)789-3930
------------------------------------
To unsubscribe from the htdig3-dev mailing list, send a message to
[EMAIL PROTECTED]
You will receive a message to confirm this.
