Here’s the current (25-aug-10) implementation of InputStream::readEntireStreamAsString:const String InputStream::readEntireStreamAsString()
{
MemoryOutputStream mo;
mo.writeFromInputStream (*this, -1);
return mo.toString();
}
It seems possible for mo.writeFromInputStream to read less than the entire stream, but since InputStream::readEntireStreamAsString doesn’t look at the return value, it doesn’t know. As things are now, it wouldn’t have anything to compare the return value to, so we need a bigger change to pull this off.
OutputStream::writeFromInputStream knows when one of its calls to InputStream::read fails. To handle the case where the caller doesn’t know how much data to expect, would you consider changing the signature of OutputStream::writeFromInputStream to something like this:[code]/** Reads data from an input stream and writes it to this stream.
@param source the stream to read from
@param maxNumBytesToWrite the number of bytes to read from the stream (if this is
less than zero, it will keep reading until the input
is exhausted)
@param numWritten if provided, populated with the number of bytes written
@returns false if either the read or write operation fails for some reason
*/
virtual bool writeFromInputStream (InputStream& source, int64 maxNumBytesToWrite, int *numWritten = 0);[/code]
I first thought ofvirtual bool writeFromInputStream (InputStream& source, int64 maxNumBytesToWrite, int &numWritten); but that requires way more changes.
With this, and something like virtual bool readEntireStreamAsString (String &contents); for InputStream::readEntireStreamAsString, callers can learn when errors have happened.
I’ll work on a patch to work this through if this seems like a reasonable way forward. Exceptions would be cleaner, but this helps in the meantime.
Those methods aren’t designed for situations where you need bulletproof error-checking, they’re just quick-and-dirty ways of grabbing the data. At the moment you can write concise things like:
I don’t have a general policy, but in this case it seems a bit pointless to change it to a less convenient format, and then eventually add exceptions and change it back again… If you need to handle an error, I’d suggest just using the writeFromInputStream call directly in your code - that’s what I tend to do.
With:MemoryOutputStream mo;
if (mo.writeFromInputStream (stream, -1))
doSomethingWith (mo.toString());doSomethingWith still gets executed even if writeFromInputStream only reads part of the underlying stream. Even ignoring the change to InputStream::readEntireStreamAsString, it seems important for the caller to learn this.
The code I was working with was XmlDocument, not my own. Changing that had a non-trivial ripple though. I suppose it was an interesting exercise to at least find out how hard it would be to handle errors elsewhere in the code. CodeDocument::loadFromStream was easy to change. Checking the return value of writeFromInputStream in XmlDocument::getDocumentElement was easy, but other changes in XmlDocument were not as obvious.
I’m not sure how File::loadFileAsString is used, but not being able to distinguish between an empty file and one that doesn’t exist seems dicey. It isn’t used in that many places but it could important in one of them. It was easy to change to handle the new readEntireStreamAsString signature. I didn’t try changing File::loadFileAsString itself.
I did change URL::readEntireTextStream so URL::readEntireXmlStream could return 0 if it failed.
If the XmlDocument’s stream fails, the parsing will fail, and it’ll return 0. It doesn’t need to actually detect the failure because that wouldn’t make any difference to its external behaviour.
