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

Shallow copy of object #3367

Closed
DartBot opened this issue Jun 3, 2012 · 8 comments
Closed

Shallow copy of object #3367

DartBot opened this issue Jun 3, 2012 · 8 comments
Labels
area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. closed-not-planned Closed as we don't intend to take action on the reported issue

Comments

@DartBot
Copy link

DartBot commented Jun 3, 2012

This issue was originally filed by [email protected]


What steps will reproduce the problem?

It seems to be unnecessary difficult to create a shallow copy of an object (please correct me if I am wrong). I couldn't find another way than to implement a method copy() in each subclass of a class hierarchy that would only call its own constructor to create a new object of the same type and with the same field values.

What is the expected output? What do you see instead?

I see that a copy() method in Object might not be desirable, but it would be very useful if Dart somehow had the possibility to create a new instance of an object with identical field values (shallow copy, clone).

What version of the product are you using? On what operating system?

Dart SDK version 7904

@madsager
Copy link
Contributor

madsager commented Jun 4, 2012

Added Area-Language, Triaged labels.

@gbracha
Copy link
Contributor

gbracha commented Jun 6, 2012

Removed Area-Language label.
Added Area-Library label.

@lrhn
Copy link
Member

lrhn commented Jun 19, 2012

I'd rather avoid having extra-linguistic ways to create an object (i.e., creating an object without calling a generative constructor). A clone method is exactly that.
Also, shallow copying isn't generally safe. It can lead objects to share otherwise private data structures, so it's only something that makes sense for objects that are actually prepared for it.

In other words, you shouldn't be able to clone an object of a class that isn't explicitly written for it.

So, if you want to write your classes for cloning, I do suggest giving them all

  Foo.clone(Foo original) : super.clone(this), ... 

constructors, and perhaps a copy method on each class that calls the constructor on the same type.

Most standard types where cloning makes sense (e.g., Set, List, Map) will eventually have some sort of copy constructor.

@DartBot
Copy link
Author

DartBot commented Jun 19, 2012

This comment was originally written by [email protected]


I agree with your argument that objects not prepared for cloning shouldn't support the operation.

I dismissed the solution you propose in my initial issue report: My problem is that I am having a huge hierarchy of classes that all must support copying. Having to implement extra constructors and copy methods in each of these classes seems to be a lot of work, add a lot of unnecessary boilerplate, and might be the cause of subtle bugs.

Ideally I would like to be able to add a single copy method to the root of my class hierarchy and let it deal with all of the sub-classes. And sub-classes would only need to override, if they actually copy some of their state. Maybe having reified classes (Issue #3368) would solve this issue as well?

@DartBot
Copy link
Author

DartBot commented Jul 16, 2012

This comment was originally written by [email protected]


I've been bitten on a number of occasions by the 'operator= creates a reference not a copy' issue. Coming from JS, that's expected. Coming from C++, it's not. Having an explicit way to perform a deep copy of an object rather than a reference to it would help a class of developers avoid really subtle bugs.

An example: A naive implementation of debug drawing in dartbox2d created local Vectors that were set to the position of objects using operator=. Drawing would change that position causing objects to unexpectedly move when certain debug drawing flags were enabled. In this case, the Vector has an existing constructor that copies the internal data. I think there should be a more general solution for this.

A 'clone' method on Object, for example, that must be overridden if operator= is overridden. Or a change to the semantics of the language such that operator= performs a deep copy and =& creates a reference to the object.

I prefer the latter as that's how my brain works, however I understand it would be a significant change.

@lrhn
Copy link
Member

lrhn commented Jul 17, 2012

I don't think there is any chance of Dart changing how objects are referenced or passed as arguments. In this respect, Dart is firmly in the same camp as, e.g., Java and ECMAScript. Objects have identity, and you can only pass around references to them. There is no implicit copying.

@lrhn
Copy link
Member

lrhn commented Apr 18, 2013

We have no current plans to make it easy to clone objects from the outside.
The objects must themselves provide the functionality, since they are the only ones that fully understand their internal invariants.


Added NotPlanned label.

@DartBot DartBot added Type-Defect area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. closed-not-planned Closed as we don't intend to take action on the reported issue labels Apr 18, 2013
@kevinvandenbreemen
Copy link

I would have to agree with the language developers' assessment here. I too am looking for a solution to automagically copying Dart objects for my own project. However after reading this and investigating something called "Mirror" (see https://stackoverflow.com/questions/17021327/how-do-i-get-all-fields-for-a-class-in-dart ) I came to the same conclusion.

I might have to write a bit more code but it'll be a heck of a lot easier to debug issues than contending with a reflective copying mechanism as well.

copybara-service bot pushed a commit that referenced this issue Mar 30, 2022
Changes:
```
> git log --format="%C(auto) %h %s" 8f5ab7b..a3a102a
 https://dart.googlesource.com/pub.git/+/a3a102a5 Fix equality and hashcode for the sdk descriptors (#3367)
 https://dart.googlesource.com/pub.git/+/94ae66a6 Refine what a relative uri means in a git path (#3212)
 https://dart.googlesource.com/pub.git/+/cc4c1292 Only call Package.listFiles once per publish. (#3346)
 https://dart.googlesource.com/pub.git/+/f4484073 Fix test/global/activate/git_package_test test on windows (#3361)
 https://dart.googlesource.com/pub.git/+/610ce7f2 Refactor descriptors (#3305)
 https://dart.googlesource.com/pub.git/+/953b6097 Substitute pub.dartlang.org for of pub.dev (#3358)
 https://dart.googlesource.com/pub.git/+/7a6ea396 Add support for pubspec overrides file (#3215)
 https://dart.googlesource.com/pub.git/+/8abfed9d Global activate git path and ref (#3356)
 https://dart.googlesource.com/pub.git/+/d1c0e3f9 Revert "Add flag controlling creation of `.packages` file. (#2757)" (#3357)
 https://dart.googlesource.com/pub.git/+/274f5ad9 Fix signals test (#3359)
 https://dart.googlesource.com/pub.git/+/83437005 Avoid failing in gitignore validator (#3354)
 https://dart.googlesource.com/pub.git/+/3082796f dependency_services: Don't download archives on apply (#3352)
 https://dart.googlesource.com/pub.git/+/48d0ffaf dependency_services: Use ^ constraints for widened intervals when possible (#3349)
 https://dart.googlesource.com/pub.git/+/826e2086 Remove obsolete test (#3347)
 https://dart.googlesource.com/pub.git/+/35e5140b Bump analyzer from 2.8.0 to 3.3.1 (#3341)
 https://dart.googlesource.com/pub.git/+/52f2bdc2 Enable dependabot (#3340)
 https://dart.googlesource.com/pub.git/+/1e70c0c7 Remove `uploader` command (#3335)
 https://dart.googlesource.com/pub.git/+/3174a264 Warn if git version is not high enough for supporting all features (#3332)
 https://dart.googlesource.com/pub.git/+/3f7a3cb7 Don't analyze ignored directories in directory-validator (#3331)
 https://dart.googlesource.com/pub.git/+/e8f36614 Allow use of token for talking to pub.dev (#3330)
 https://dart.googlesource.com/pub.git/+/b93bf88f Upgrade `package:tar` to version `0.5.4`. (#3313)
 https://dart.googlesource.com/pub.git/+/fbc9732e Support for different versioning strategies in dependency_services (#3320)
 https://dart.googlesource.com/pub.git/+/93c7cfcd Update repository-spec-v2.md (#3311)
 https://dart.googlesource.com/pub.git/+/941191f7 dependency_services (#3304)
 https://dart.googlesource.com/pub.git/+/61175cb6 fix: relative to the current directory rules (#3297)
 https://dart.googlesource.com/pub.git/+/f27e90d3 Upgrade other versions conservatively with --major-versions (#3295)
 https://dart.googlesource.com/pub.git/+/a2461417 Add flag controlling creation of `.packages` file. (#2757)

```

Diff: https://dart.googlesource.com/pub.git/+/8f5ab7b1aba3b9f66b56246d77e167990339d317~..a3a102a549388a6dbfecc9252fabb618f9a2f5f7/
Change-Id: I8d0ea375039ea450d397871d9fac35d590ea8869
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/239315
Reviewed-by: Jonas Jensen <[email protected]>
Commit-Queue: Sigurd Meldgaard <[email protected]>
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. closed-not-planned Closed as we don't intend to take action on the reported issue
Projects
None yet
Development

No branches or pull requests

5 participants