Forum Navigation

Console.readln issue

Moderators: kris

korDen

Joined: 03/12/08

Posts: 13
Posted: 08/28/08 18:15:57 Modified: 08/28/08 18:16:54
It's a common pattern (and a good one!) in D for a function that returns a string to accept an optional parameter which is used as a buffer. It then returns a slice of it, if the buffer is big enough, and allocates additional memory, if it isn't. Right?

Here is an example:
import tango.io.Stdout;

// helper function, shouldn't it be somewhere in Tango?
void swap(T)(ref T a, ref T b)
{
    T tmp = a;
    a = b;
    b = tmp;
}

// helper function that reverses an array in-place. Why didn't I find one in tango.text.Util?
T[] reverse(T)(T[] arr)
{
    int length = arr.length;
    for (int begin = 0, end = length-1; begin < end; ++begin, --end) {
        swap(arr[begin], arr[end]);
    }

    return arr;
}

char[] toString(int i, char[] tmpBuffer = null)
{
    int bufferLength = tmpBuffer.length;
    int realLength = 0;

    void appendChar(char c) {
        if (realLength < bufferLength) {
            tmpBuffer[realLength] = c;
        } else {
            tmpBuffer ~= c;
        }
        ++realLength;
    }

    do {
        char digit = '0' + i % 10;
        appendChar(digit);

        i /= 10;
    } while (i != 0);
    
    return tmpBuffer[0..realLength].reverse();
}

void main()
{
    char[] buffer = new char[3];
    Stdout(toString(12345, buffer)).newline;
}
From this point of view, I think that Console.readln method can be improved, because its signature is as follows:
bool readln (inout char[] content, bool raw=false);
My intuition says me that I should apply the pattern described above to it:
char[] buffer = new char[1024];
bool success = Console.readln(buffer);
if (success) {
    Stdout(buffer);
}
I.e. 1) buffer may be created to reduce allocations (but not necessary) 2) buffer owns the data, i.e. there is no need to .dup it.

None of the above points are valid for readln(). And I am not the only one who is confused by actual behaviour.

That's why I suggest to change (or create an overload) it with the following one:
char[] readln (bool raw=false)
{
    char[] content;
    bool success = readln (content, raw);
    if (!success) {
        return null; // note that (null !is "")
        // or throw new Exception("Source exhausted");
    }

    return content;
}
It is much simpler, more intuitive and no-one is claiming that buffer ougth to be preallocated and returned data is owned by user.

What do you think?

Author Message
korDen

Joined: 03/12/08

Posts: 13
Posted: 08/28/08 18:16:35

BTW, in a multithreaded environment, the returned data might be a garbage, if two threads are using readln at the same time even if the method is syncronized:

synchronized char[] readln(bool raw = false) { char[] content; bool success = readln (content, raw); if (!success) { return null; // note that (null !is "") // or throw new Exception("Source exhausted"); } return content; }
Scenario: 1st thread executes readln and stores in a temporary variable 2nd thread executes readln and stores in a temporary variable. Internal buffer is used and overwritten, thus the slice referenced by thread one is invalid.

readln().dup() doesn't help either.

Your thoughts?
kris

Joined: 03/27/04

Posts: 581
Posted: 08/29/08 06:19:58

the purpose of readln() is to expose a slice of console input, not a copy, duplicate, or anything else. You don't preallocate anything for it either ... instead it is used like so:

char[] line; while (Cin.readln(line)) // do something with line
And yes, it is a little different to the strategy Tango uses for much of the text processing library. In this particular case it was deemed to be ok. Note that the above example has no heap activity at all, deliberately so, in order to make redirected console processing efficient. Perhaps your suggestion might be more apropos for copyln() ?
korDen

Joined: 03/12/08

Posts: 13

Posted: 08/31/08 12:02:21

Yeah, thanks! How come I didn't notice the copyln()?