On 4/9/12, Kevin Cox <kevincox.ca@gmail.com> wrote:
> The
> reason I aski is because if you have a return statement inside a foreach it
> returns from the outside function not the "closure".

I don't like this subtle thing. For example let's say a newbie were to implement a function called "isDirEmpty". The first hint is to try to walk the directory iteratively and return as soon as there's a match, e.g.:

bool isEmptyDir(string path)
{
    foreach (_; dirEntries(path, SpanMode.shallow))
        return false;

    return true;
}

isEmptyDir never returns false. All that false statement does is break out of the foreach loop.

Of course the right way to implement this is:

bool isEmptyDir(string path)
{
    return dirEntries(path, SpanMode.shallow).empty;
}

But still, the first version is extremely subtle. The semantics of that code completely change based on whether dirEntries is an array, a range, or an opApply.

On 10.04.2012 13:33, Andrej Mitrovic wrote:
> On 4/9/12, Kevin Cox<kevincox.ca@gmail.com>  wrote:
>> The
>> reason I aski is because if you have a return statement inside a foreach it
>> returns from the outside function not the "closure".
>
> I don't like this subtle thing. For example let's say a newbie were to
> implement a function called "isDirEmpty". The first hint is to try to
> walk the directory iteratively and return as soon as there's a match,
> e.g.:
>
> bool isEmptyDir(string path)
> {
>      foreach (_; dirEntries(path, SpanMode.shallow))
>          return false;
>
>      return true;
> }
>
> isEmptyDir never returns false. All that false statement does is break
> out of the foreach loop.

Wake up! dirEntries produce a lazy _range_ and it's not opApply & closures. And you were the one to come up with this cool parallel(dirEntries(...)) thing back when it was introduced.

Anyway that "return" false would have been translated to some "black-magic" jump.

>
> Of course the right way to implement this is:
>
> bool isEmptyDir(string path)
> {
>      return dirEntries(path, SpanMode.shallow).empty;
> }
>
> But still, the first version is extremely subtle. The semantics of
> that code completely change based on whether dirEntries is an array, a
> range, or an opApply.

array vs range is the same here. opApply is the weirdo.

-- 
Dmitry Olshansky

On 4/10/12, Dmitry Olshansky <dmitry.olsh@gmail.com> wrote:
> Wake up! dirEntries produce a lazy _range_ and it's not opApply

Sorry? Change DirIterator in std.file to this: http://pastebin.com/DHvXuFeH

test.d:
import std.file;

bool isEmptyDir(string path)
{
    foreach (string name; dirEntries(path, SpanMode.shallow))
        return false;

    return true;
}

void main()
{
    isEmptyDir(".");
}

$ rdmd test.d
> we're in oppapply

On 04/10/2012 11:33 AM, Andrej Mitrovic wrote:
> On 4/9/12, Kevin Cox<kevincox.ca@gmail.com>  wrote:
>> The
>> reason I aski is because if you have a return statement inside a foreach it
>> returns from the outside function not the "closure".
>
> I don't like this subtle thing. For example let's say a newbie were to
> implement a function called "isDirEmpty". The first hint is to try to
> walk the directory iteratively and return as soon as there's a match,
> e.g.:
>
> bool isEmptyDir(string path)
> {
>      foreach (_; dirEntries(path, SpanMode.shallow))
>          return false;
>
>      return true;
> }
>
> isEmptyDir never returns false.

Yes it does.

> All that false statement does is break
> out of the foreach loop.
>

??? No, it returns false from isEmptyDir, unless the opApply implementation abuses operator overloading.

On 04/10/2012 12:06 PM, Andrej Mitrovic wrote:
> On 4/10/12, Dmitry Olshansky<dmitry.olsh@gmail.com>  wrote:
>> Wake up! dirEntries produce a lazy _range_ and it's not opApply
>
> Sorry? Change DirIterator in std.file to this:
> http://pastebin.com/DHvXuFeH
>

You are doing it wrong.
Use:
if(auto r=dg(s)) return r;
instead.

> test.d:
> import std.file;
>
> bool isEmptyDir(string path)
> {
>      foreach (string name; dirEntries(path, SpanMode.shallow))
>          return false;
>
>      return true;
> }
>
> void main()
> {
>      isEmptyDir(".");
> }
>
> $ rdmd test.d
>> we're in oppapply

On 04/10/2012 12:47 PM, Timon Gehr wrote:
> On 04/10/2012 12:06 PM, Andrej Mitrovic wrote:
>> On 4/10/12, Dmitry Olshansky<dmitry.olsh@gmail.com> wrote:
>>> Wake up! dirEntries produce a lazy _range_ and it's not opApply
>>
>> Sorry? Change DirIterator in std.file to this:
>> http://pastebin.com/DHvXuFeH
>>
>
> You are doing it wrong.
> Use:
> if(auto r=dg(s)) return r;
> instead.
>

WTF??! It is actually wrong in Phobos. Bug!

On 04/10/2012 12:50 PM, Timon Gehr wrote:
> On 04/10/2012 12:47 PM, Timon Gehr wrote:
>> On 04/10/2012 12:06 PM, Andrej Mitrovic wrote:
>>> On 4/10/12, Dmitry Olshansky<dmitry.olsh@gmail.com> wrote:
>>>> Wake up! dirEntries produce a lazy _range_ and it's not opApply
>>>
>>> Sorry? Change DirIterator in std.file to this:
>>> http://pastebin.com/DHvXuFeH
>>>
>>
>> You are doing it wrong.
>> Use:
>> if(auto r=dg(s)) return r;
>> instead.
>>
>
> WTF??! It is actually wrong in Phobos. Bug!

http://d.puremagic.com/issues/show_bug.cgi?id=7884

On 10.04.2012 14:50, Timon Gehr wrote:
> On 04/10/2012 12:47 PM, Timon Gehr wrote:
>> On 04/10/2012 12:06 PM, Andrej Mitrovic wrote:
>>> On 4/10/12, Dmitry Olshansky<dmitry.olsh@gmail.com> wrote:
>>>> Wake up! dirEntries produce a lazy _range_ and it's not opApply
>>>
>>> Sorry? Change DirIterator in std.file to this:
>>> http://pastebin.com/DHvXuFeH
>>>
>>
>> You are doing it wrong.
>> Use:
>> if(auto r=dg(s)) return r;
>> instead.
>>
>
> WTF??! It is actually wrong in Phobos. Bug!

Is it still opApply in Phobos?
I mean I did the pull that removed it.

-- 
Dmitry Olshansky

On 10.04.2012 14:06, Andrej Mitrovic wrote:
> On 4/10/12, Dmitry Olshansky<dmitry.olsh@gmail.com>  wrote:
>> Wake up! dirEntries produce a lazy _range_ and it's not opApply
>
> Sorry? Change DirIterator in std.file to this:
> http://pastebin.com/DHvXuFeH
>

Yeah recent change allowed to use straight alias this in DirEntry struct thus there is no need for separate opApply now.

> test.d:
> import std.file;
>
> bool isEmptyDir(string path)
> {
>      foreach (string name; dirEntries(path, SpanMode.shallow))
>          return false;
>
>      return true;
> }
>
> void main()
> {
>      isEmptyDir(".");
> }
>
> $ rdmd test.d
>> we're in oppapply


-- 
Dmitry Olshansky

On 04/10/2012 12:54 PM, Dmitry Olshansky wrote:
> On 10.04.2012 14:50, Timon Gehr wrote:
>> On 04/10/2012 12:47 PM, Timon Gehr wrote:
>>> On 04/10/2012 12:06 PM, Andrej Mitrovic wrote:
>>>> On 4/10/12, Dmitry Olshansky<dmitry.olsh@gmail.com> wrote:
>>>>> Wake up! dirEntries produce a lazy _range_ and it's not opApply
>>>>
>>>> Sorry? Change DirIterator in std.file to this:
>>>> http://pastebin.com/DHvXuFeH
>>>>
>>>
>>> You are doing it wrong.
>>> Use:
>>> if(auto r=dg(s)) return r;
>>> instead.
>>>
>>
>> WTF??! It is actually wrong in Phobos. Bug!
>
> Is it still opApply in Phobos?
> I mean I did the pull that removed it.
>

Yes, it is fixed in git head. Sorry for the noise.