language icon indicating copy to clipboard operation
language copied to clipboard

Deprecate `new` keyword WRT `new SomeClass`

Open kevmoo opened this issue 2 years ago • 10 comments

Unlike const, there are no places (that I can think of) where this would still be needed to instantiate a type.

kevmoo avatar Aug 10 '22 19:08 kevmoo

The new keyword is convenient to people coming from other languages like Java, C# and TypeScript. The removal of it may not be much beneficial to the language.

Wdestroier avatar Aug 11 '22 05:08 Wdestroier

I sincerely hope that new is not deprecated. I personally like it (and we enforce the use of it in the front_end) because it becomes very clear when we do an allocation.

For instance, this runs in ~600 ms on my machine:

class foo {
  final List<int> _manyInts = [];
  foo() {
    for(int i = 0; i < 10000; i++) {
      _manyInts.add(i);
    }
  }

  bool has(int i) {
    return _manyInts.contains(i);
  }
}

int process(List<int> data) {
  int has = 0;
  for(int entry in data) {
    if (foo().has(entry)) {
      has++;
    }
  }
  return has;
}

void main() {
  List<int> data = List<int>.generate(10000, (index) => index * 3);
  Stopwatch stopwatch = new Stopwatch()..start();
  int had = process(data);
  print("Got result $had in ${stopwatch.elapsedMilliseconds} ms");
}

It would be a lot easier to see that

  for(int entry in data) {
    if (foo().has(entry)) {
      has++;
    }
  }

was probably a bad idea if it has been

  for(int entry in data) {
    if (new foo().has(entry)) {
      has++;
    }
  }

It would have made it easier for me to see that I did something I shouldn't when writing it, and it would make it easier for any reviewer to spot when doing the review (and moving it out of the loop and only doing a single allocation makes it run more than 6 times as fast).

Please let's keep the status quo and allow those of us who like the new to keep it!

jensjoha avatar Aug 15 '22 12:08 jensjoha

@jensjoha Only if you ignore camel_case_types and non_constant_identifier_names, which I wouldn't recommend.

Otherwise, Foo() would be clearly an object instantiation.

mateusfccp avatar Aug 15 '22 12:08 mateusfccp

@mateusfccp We have named constructors whose invocations look like static method invocation, so the problem persists even when you follow naming conventions.

johnniwinther avatar Aug 15 '22 12:08 johnniwinther

@jensjoha Only if you ignore camel_case_types and non_constant_identifier_names, which I wouldn't recommend.

Otherwise, Foo() would be clearly an object instantiation.

I did make Foo lower case to make it less obvious here, but I disagree that it's clearly anything --- to me It's a lot easier to see new Foo than to see Foo.

jensjoha avatar Aug 15 '22 12:08 jensjoha

It also helps in showing who owns an object. With the new keyword it is locally visible that the object is not shared with anyone. Without new you have to determine (guess) this from the name.

johnniwinther avatar Aug 15 '22 12:08 johnniwinther

The underlying debate could perhaps be described as follows: Do we insist on using concise forms? Or do we allow developers (ourselves and others) to use human judgment, e.g., choosing a slightly more verbose form in order to document properties of the software?

I think the distinction between an instance creation Foo.bar() and a static method invocation Foo.bar() could be useful to document, perhaps because the former is costly and the latter is fast, or because it is important to maintain that the former is a fresh object each time it is evaluated (OK, factories can violate that, but "informative" doesn't have to imply "without exception").

The response could be "just write a comment", which is of course a very general approach, and the ability to have or not have new in front of Foo.bar() isn't particularly expressive or flexible. There are many other things we might want to be able to specify. However, language mechanisms have the added benefit of being checked mechanically all the time.

All in all, I'd prefer if we allow ourselves to use the few such devices that we have, when it improves code readability.

eernstg avatar Aug 15 '22 15:08 eernstg

it becomes very clear when we do an allocation.

Factory constructor has entered the chat.

The new keyword doesn't mean anything in Dart. It's just another kind of static invocation. The presence of a new keyword doesn't strictly imply allocation, and its absence doesn't imply a lack of allocation. A team could by convention, decide to forbid factory constructors and then be assured that any new strictly implied an allocation.

But even that's not that helpful. Any place you don't see new could also be doing allocation. It's not like any random instance or static method can't internally allocate to its heart's content:

main() {
  // Looks fast and memory efficient!
  print(superFastVariableAccess);
}

bool get superFastVariableAccess {
  // Oops! Definitely not fast or memory efficient.
  return List<String>.generate(1000000, (i) => i.toString()).isEmpty;
}

Dart is, by design, quite programmable. Almost all syntax in the language can invoke arbitrary user-defined code:

  • Factory constructors mean new can do anything a function can do.
  • Even generative constructors let you run arbitrary code after allocating the instance.
  • Almost all infix operators can be overloaded.
  • For-in loops can invoke user-defined implementations of Iterable.
  • Classes can implement call() and be used as functions.
  • Getters and setters look like field accessors but run arbitrary code.
  • You can even have top-level getters and setters that look like variables, not properties of objects.
  • The subscript operator and subscript set operators let [] run user defined code even when on the left hand side of =.

Dart is simply not designed to let you reason accurately about performance by looking at it syntactically. That's what profilers are for.

munificent avatar Aug 16 '22 23:08 munificent

I find it hard to understand why leaving the choice of using new is seemingly stepping on so many toes. To me code is more readable and easier to understand if there's a new in front of an allocation. I'm not saying that you have to use it in your code. I'm saying that I want it in mine.

For your example:

main() {
  // Looks fast and memory efficient!
  print(superFastVariableAccess);
}

bool get superFastVariableAccess {
  // Oops! Definitely not fast or memory efficient.
  return List<String>.generate(1000000, (i) => i.toString()).isEmpty;
}

...again the new would have made it more readable to me:

bool get superFastVariableAccess {
  // Oops! Definitely not fast or memory efficient.
  return new List<String>.generate(1000000, (i) => i.toString()).isEmpty;
}

Dart is simply not designed to let you reason accurately about performance by looking at it syntactically.

Even if Dart is not designed for it, that doesn't mean we have to make it harder to actually do it.

That's what profilers are for.

In my opinion this is very backwards. We shouldn't aim to correct bad performing code, we should aim to write good performing code from the start. Obviously we can't always do that, but it should be our aim, just like we aim to write correct code up front instead of always relying on using our debugger after the fact. We can't always do that either, but we try.

Also, going to look via a profiler first means that we have to notice something bad going on, which we might not if it adds, say, 500 ms in something that takes, say, 30 seconds. That doesn't mean that we should just waste those 500 ms though --- such waste will add up, and down the line we'll just have a very slow product that has lots of little things wrong with it, making it very hard to profile your way out of.

Again: new helps me. I'm not claiming that it helps you, but it does help me. Let me keep using it.

jensjoha avatar Aug 17 '22 06:08 jensjoha

We shouldn't aim to correct bad performing code, we should aim to write good performing code from the start.

I agree completely, but the only way to get there is to know semantically what code you're calling into. The syntax just doesn't tell you anything meaningful in the way that it does in, say, C++. Even if you see a new, that doesn't tell you that an allocation is occurring. And if you don't see new, that doesn't mean an allocation isn't occurring.

It's, of course, your code, and you're free to write it in whatever style you prefer. But we recommend a style that we think produces the best, most readable code for the greatest number of users, and we encourage all people to follow that style because consistent style also makes code easier to read.

munificent avatar Aug 17 '22 21:08 munificent

I never use it, don't see the need for it, but if others like it that's cool.

I'd just like to be able to use it as a function name, that's all. Right now this perfectly succinct, and semantically accurate description is impossible:

class Foo {
  final String a;
  final String b;
  final String c;
  final int d;
  final X e;
  final X? f;

  const Foo({
    this.a = '',
    required this.b,
    required this.c,
    required this.d,
    required this.e,
    this.f,
  });
  
  Foo new({String? a, String? b, String? c, int? d, X? e, X? f}) =>
      Foo(
        a: a?? this.a,
        b: b ?? this.b,
        c: c ?? this.c,
        d: d ?? this.d,
        e: e ?? this.e,
        f: f ?? this.f,
      );
}

So what does flutter often do for this functionality? the godawful .copyWith(). Maybe I'm a semantics snob or maybe this would be really nice, either way, case in point.

lastmeta avatar Feb 10 '23 21:02 lastmeta

I'd just like to be able to use it as a function name.

That wouldn't happen anyway, it'd conflict with the ClassName.new constructor tear-off syntax.

lrhn avatar Feb 18 '23 19:02 lrhn