Details

    • Type: Improvement Improvement
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: None
    • Labels:
      None

      Description

      Feedback on addArray methods: "API already has beginArray() for starting a new array, better to turn this into addAll(Iterable<JsonValue>) which would add all the elements of the iterable at the current point. That way addAll() can do double-duty both to bulk-add into the currently built array, or you can do .beginArray().addAll(iterable).endArray() to add a nested array."

        Activity

        Hide
        jitu added a comment -

        Proposed changes are:

        Remove
        JsonArrayBuilder#addArray(Iterable<JsonValue>)
        JsonObjectBuilder#addArray(String, Iterable<JsonValue>)

        and add
        JsonArrayBuilder#addAll(Iterable<JsonValue>)

        Show
        jitu added a comment - Proposed changes are: Remove JsonArrayBuilder#addArray(Iterable<JsonValue>) JsonObjectBuilder#addArray(String, Iterable<JsonValue>) and add JsonArrayBuilder#addAll(Iterable<JsonValue>)
        Hide
        illsleydc added a comment -

        I definitely agree with the need for an addAll to JsonArrayBuilder, but I think it's worth exploring further and adding a few... (I don't know where a developer would normally be getting the Iterable<JsonValue> from?)

        addAll(Iterable<String>), addAll(Iterable<Integer>), addAll(Iterable<Double>)

        The obvious inefficiency here is for people with int[] or double[]. For them, addAll(int...) etc would be better. That could get to be a lot of methods, but with consistent naming, it might not feel too overwhelming.

        Show
        illsleydc added a comment - I definitely agree with the need for an addAll to JsonArrayBuilder, but I think it's worth exploring further and adding a few... (I don't know where a developer would normally be getting the Iterable<JsonValue> from?) addAll(Iterable<String>), addAll(Iterable<Integer>), addAll(Iterable<Double>) The obvious inefficiency here is for people with int[] or double[]. For them, addAll(int...) etc would be better. That could get to be a lot of methods, but with consistent naming, it might not feel too overwhelming.
        Hide
        jitu added a comment -

        > I definitely agree with the need for an addAll to JsonArrayBuilder, but I think it's worth exploring further and adding a few... (I don't know where a developer would normally be getting the Iterable<JsonValue> from?)

        The developer may get Iterable<JsonValue> from an existing JsonArray, say JsonArray#getValues().

        >addAll(Iterable<String>), addAll(Iterable<Integer>), addAll(Iterable<Double>)
        >The obvious inefficiency here is for people with int[] or double[]. For them, addAll(int...) etc would be better. That could get to be a lot of methods, but with consistent naming, it might not feel too overwhelming.

        We already have methods to add one JsonValue, int, String etc. Perhaps, we just remove the current addArray() methods and not add any new addAll methods. A developer can add them using the existing methods in a loop. If the addAll methods really needed, we can add them later.

        Show
        jitu added a comment - > I definitely agree with the need for an addAll to JsonArrayBuilder, but I think it's worth exploring further and adding a few... (I don't know where a developer would normally be getting the Iterable<JsonValue> from?) The developer may get Iterable<JsonValue> from an existing JsonArray, say JsonArray#getValues(). >addAll(Iterable<String>), addAll(Iterable<Integer>), addAll(Iterable<Double>) >The obvious inefficiency here is for people with int[] or double[]. For them, addAll(int...) etc would be better. That could get to be a lot of methods, but with consistent naming, it might not feel too overwhelming. We already have methods to add one JsonValue, int, String etc. Perhaps, we just remove the current addArray() methods and not add any new addAll methods. A developer can add them using the existing methods in a loop. If the addAll methods really needed, we can add them later.
        Hide
        jitu added a comment -

        Removing the addArray methods on the builders.

        Show
        jitu added a comment - Removing the addArray methods on the builders.

          People

          • Assignee:
            jitu
            Reporter:
            jitu
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: