Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add ref to array() and object() of JSONValue getters to add new element #1916

Merged
merged 2 commits into from Feb 10, 2014

Conversation

repeatedly
Copy link
Member

Continuation of #1421

New JSONValue API can't add new element to array or object directly.
Making array / object getter ref return fuction fix this problem.

@ghost
Copy link

ghost commented Feb 9, 2014

Please add test-cases.

@repeatedly
Copy link
Member Author

@AndrejMitrovic added

@ghost
Copy link

ghost commented Feb 9, 2014

So essentially for the array case this depends on the following behavior:

import std.stdio;

struct S
{
    /// getter
    @property ref int[] array()
    {
        writeln("getter");
        return _array;
    }

    /// setter
    @property int[] array(int[] v)
    {
        writeln("setter");
        return _array;
    }

    int[] _array;
}

void main()
{
    S s;
    s.array = [1];   // calls setter
    s.array ~= [1];  // calls getter
}

I'm not sure whether this is appropriate or whether we should use operator overloading here? What do others think, @9rnsr, @CyberShadow ?

I think the code is still correct though, since the getter does type checking anyway.

@CyberShadow
Copy link
Member

It looks fine on the surface. One potential issue is that it does not allow building an array from zero, as the first append will call the getter, but that's not different from the equivalent JavaScript code (arr.push(value) when arr is undefined).

@9rnsr
Copy link
Contributor

9rnsr commented Feb 10, 2014

Looks good to me.

@9rnsr
Copy link
Contributor

9rnsr commented Feb 10, 2014

Auto-merge toggled on

9rnsr added a commit that referenced this pull request Feb 10, 2014
…-getter

Add ref to array() and object() of JSONValue getters to add new element
@9rnsr 9rnsr merged commit 8a1fc12 into dlang:master Feb 10, 2014
9rnsr added a commit to 9rnsr/phobos that referenced this pull request Feb 15, 2014
…bject-getter

Add ref to array() and object() of JSONValue getters to add new element
@9rnsr
Copy link
Contributor

9rnsr commented Feb 15, 2014

I think the fixed issue is a regression introduced in the 2.065 development.
https://d.puremagic.com/issues/show_bug.cgi?id=12168

So I think we should cherry-pick it in 2.065 release branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants