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

Why deepCopy is not public? #760

Closed
kramer opened this issue Dec 26, 2015 · 27 comments
Closed

Why deepCopy is not public? #760

kramer opened this issue Dec 26, 2015 · 27 comments

Comments

@kramer
Copy link

kramer commented Dec 26, 2015

Is there a reason why the deepCopy methods are not public? I see that they were public for some time in an earlier revision then was made "default" access level but I couldn't find the reason.

@inder123
Copy link
Collaborator

It was never made public in a released version. No specific reason, we just didn't see it useful enough to bloat our API. What are you trying to do that you need it?

@kramer
Copy link
Author

kramer commented Dec 26, 2015

It was never made public in a released version.

Fair enough, I was only checking the file history and didn't notice that, sorry.

What are you trying to do that you need it?

To be exact for this but a shorter version is:

class Demo {
  private JsonObject serverResponse;

  public JsonObject getResultsFromResponse() {
    JsonObject results = serverResponse.getAsJsonObject("results");
    results.add("SOME_DEFAULT_DATA", "");
    return results;
  }
}

In this code piece calling getResultsFromResponse will change content of the class variable serverResponse but actually what I needed was to add the default data field only to the object returned by that method. So what I wanted to do is create a copy of results then add the default field to it and return so that serverResponse would not be affected - but this is not possible without access to deepCopy.

Let me know if I overlooked something or if this is possible to do without copying.

@JakeWharton
Copy link
Contributor

We could support deep copy by exposing clone() with a covariant return type
on these types. We do this in OkHttp and Retrofit to great effect (not that
I'm biased towards those libraries or anything).

On Sat, Dec 26, 2015, 12:36 PM Cihat Keser notifications@github.com wrote:

It was never made public in a released version.

Fair enough, I was only checking the file history and didn't notice that,
sorry.

What are you trying to do that you need it?

To be exact for this
https://github.com/searchbox-io/Jest/blob/master/jest-common/src/main/java/io/searchbox/client/JestResult.java#L203
but a shorter version is:

class Demo {
private JsonObject serverResponse;

public JsonObject getResultsFromResponse() {
JsonObject results = serverResponse.getAsJsonObject("results");
results.add("SOME_DEFAULT_DATA", "");
return results;
}
}

In this code piece calling getResultsFromResponse will change content of
the class variable serverResponse but actually what I needed was to add
the default data field only to the object returned by that method. So what
I wanted to do is create a copy of results then add the default field to
it and return so that serverResponse would not be affected - but this is
not possible without access to deepCopy.

Let me know if I overlooked something or if this is possible to do without
copying.


Reply to this email directly or view it on GitHub
#760 (comment).

@swankjesse
Copy link
Collaborator

Two things:

  • I think it’s conventional for mutable value objects like this to have clone() return a shallow copy. So we’d probably want to name the method deepCopy(), and possibly also have clone() that does a shallow copy. (Reason being, the default Object.clone() is a shallow copy).
  • It’s possible to create a mutable JsonPrimitive by creating it with an AtomicInteger. If we expose a mechanism to deep copy, we should probably convert numbers to LazilyParsedNumber instances.

@swankjesse
Copy link
Collaborator

And for what it’s worth, a shallow copy is sufficient to satisfy @kramer’s request.

@kramer
Copy link
Author

kramer commented Dec 26, 2015

And for what it’s worth, a shallow copy is sufficient to satisfy @kramer’s request.

@swankjesse May be true for the exact code I posted but are you sure that a shallow copy is still sufficient if the modification (the field addition) was in a deeper level (e.g.: results.getAsJsonObject("failed").add(...) - here the failed object will still be shared between the class var and the returned copy if I'm not mistaken) ?

@swankjesse
Copy link
Collaborator

Yep, in that case you’ll need a deep copy.

@sinaci
Copy link

sinaci commented Apr 26, 2016

I came across this thread when I encountered the fact that I cannot extend JsonElement because of the default access level of the deepCopy. Is there any alternative for me? I want to implement my own class which extends JsonElement.

@sheng91leo
Copy link

Hi @kramer, so far have you found any solution to solve the issue? I'm right facing the same issue as you did..

@balajeetm
Copy link

Hi,

I'm looking at ways to merge Jsons the way its available in lodash.
I couldn't find it with Gson. Is it available?
If not, I'm planning to write one. DeepCopy would be helpful here.
Can this is be made public?

Regards,

@inder123
Copy link
Collaborator

No specific reason to hide deepCopy (except to have a smaller API set).
For now, you can copy over the code?

@sheng91leo
Copy link

sheng91leo commented Sep 19, 2016

@inder123 now i did the deepCopy in this way for Gson
`/**
* This method makes a "deep copy" of JsonElement it is given.
*/

public static JsonElement deepClone(JsonElement element) {
    try {
        String elementString = element.toString();
        return new JsonParser().parse(elementString);
    }
    catch (Exception e) {
        e.printStackTrace();
        return null;
    }
}`

So far it works for what I need, but I'm not sure whether it is good in term of process performance.

@khoanguyen123
Copy link

+1 for making deepCopy() public. My code below would alter the source, which I'd like to prevent:

private JsonElement removeAttribute(String attribute, JsonObject source) {
    source.remove(attribute);
    return source;
}

@artfiedler
Copy link

+1 deepCopy() should be public, I would use it when merging two JsonObject's into one when the JsonObjects are settings and default settings... like so...

public static JsonObject merge(String overrideJson, JsonObject defaultObj) {
    return mergeInto((JsonObject)new JsonParser().parse(overrideJson), defaultObj);
}
public static JsonObject merge(JsonObject overrideObj, JsonObject defaultObj) {
    return mergeOverride((JsonObject)deepClone(defaultObj), overrideObj);
}
public static JsonObject mergeOverride(JsonObject targetObj, JsonObject overrideObj) {
    for (Map.Entry<String, JsonElement> entry : overrideObj.entrySet())
            targetObj.add(entry.getKey(), deepClone(entry.getValue()));
    return targetObj;
}
public static JsonObject mergeInto(JsonObject targetObj, JsonObject defaultObj) {
    for (Map.Entry<String, JsonElement> entry : defaultObj.entrySet()) {
        if (targetObj.has(entry.getKey()) == false)
            targetObj.add(entry.getKey(), deepClone(entry.getValue()));
    }
    return targetObj;
}
public static JsonElement deepClone(JsonElement el){
    if (el.isJsonPrimitive() || el.isJsonNull())
        return el;
    if (el.isJsonArray()) {
        JsonArray array = new JsonArray();
        for(JsonElement arrayEl: el.getAsJsonArray())
            array.add(deepClone(arrayEl));
        return array;
    }
    if(el.isJsonObject()) {
        JsonObject obj = new JsonObject();
        for (Map.Entry<String, JsonElement> entry : el.getAsJsonObject().entrySet()) {
            obj.add(entry.getKey(), deepClone(entry.getValue()));
        }
        return obj;
    }
    throw new IllegalArgumentException("JsonElement type " + el.getClass().getName());
}

@mykhaylo-
Copy link

+1 to make the deepCopy public. This feature is naturally expected to be provided by the gson library. There are lot of cases where you just want to take some your JsonObject as a source (or template) for your further manipulations(adding/removing properties, etc) but keeping the original untouched. with deepCopy you don't need to reinvent a wheel in every of your projects. you simply use it out of the box.

@bohdaq
Copy link

bohdaq commented Nov 3, 2016

Guys please make deepCopy public...

@GeniusWiki
Copy link

yes, it needs a way to clone. I prefer just implements Cloneable interface and just make it deepCopy.

@inder123
Copy link
Collaborator

inder123 commented Nov 26, 2016 via email

@CateyesLin
Copy link

+1
I need JsonElement make deepCopy public for doing merge/extend 2 JsonObject.

@adaviding
Copy link

+1 make it public

@thinkingmedia
Copy link

Do it already...

@tianhao
Copy link

tianhao commented May 16, 2017

+1 need public

@Ankhwatcher
Copy link

Ankhwatcher commented May 29, 2017

+1 My life would be easier if this were public.
I need to perform a large number of String substitutions to a copy of my JsonObject (and it's many children etc) while leaving the original as I found it.

Here's my workaround solution:

public class GsonUtils {

    public static JsonObject deepCopy(JsonObject jsonObject) {
        JsonObject result = new JsonObject();

        for (Map.Entry<String, JsonElement> entry : jsonObject.entrySet()) {
            if (entry.getValue().isJsonObject()) {
                result.add(entry.getKey(), deepCopy(entry.getValue().getAsJsonObject()));
            } else if (entry.getValue().isJsonArray()) {
                result.add(entry.getKey(), deepCopy(entry.getValue().getAsJsonArray()));
            } else if (entry.getValue().isJsonPrimitive()) {
                result.add(entry.getKey(), new JsonPrimitive(entry.getValue().getAsString()));
            }

        }

        return result;
    }

    private static JsonArray deepCopy(JsonArray jsonArray) {
        JsonArray result = new JsonArray();

        for (JsonElement jsonElement : jsonArray) {
            if (jsonElement.isJsonObject()) {
                result.add(deepCopy(jsonElement.getAsJsonObject()));
            } else if (jsonElement.isJsonArray()) {
                result.add(deepCopy(jsonElement.getAsJsonArray()));
            } else if (jsonElement.isJsonPrimitive()) {
                result.add(new JsonPrimitive(jsonElement.getAsString()));
            }
        }

        return result;
    }
}

@inder123
Copy link
Collaborator

#1091

@djdance
Copy link

djdance commented Feb 19, 2018

very useful function, will wait for it

@faissaloo
Copy link

It's public now, please close.

@Marcono1234
Copy link
Collaborator

@eamonnmcmanus, this was resolved by #1091.

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

No branches or pull requests