14

I've been writing some code in Dart. I really love the factory constructor, but I'm afraid that I'm abusing it's usefulness. In particular, when I write a value object class, I sometimes return null if the validation fails.

class EmailAddress {
  static final RegExp _regex = new RegExp(...);
  final String _value;

  factory EmailAddress(String input) {
    return _regex.hasMatch(input) ? new EmailAddress._internal(input) : null;
  }

  const EmailAddress._internal(this._value);

  toString() => _value;
}

At first, this doesn't seem all that bad. However, when you actually use it, this is what you see.

methodThatCreatesAnEmailAddress() {
  var emailAddress = new EmailAddress("definitely not an email address");
  ...
}

The argument why this is bad is that a developer coming from another statically typed language, such as Java or C++, would expect emailAddress to always be initialized to a non-null value. The argument why this is perfectly acceptable is that the constructor is factory and, as such, is allowed to return a null value.

So is this bad practice or taking advantage of a useful feature?

Günter Zöchbauer
  • 623,577
  • 216
  • 2,003
  • 1,567
Andrew
  • 603
  • 1
  • 5
  • 13
  • `return _regex.hasMatch(input) ? new EmailAddress._internal(input) : throw "something went wrong";` Also your "would expect emailAddress to always be initialized to a non-null value" is not always possible if you expected the correct result from wrong passed arguments. – mezoni Feb 15 '14 at 10:57
  • the validation of the email adress is not to be done in the constructor, but rather right when you ask it to the user. Validating as soon as possible, and informing user as soon as possible (for example, by disabling the 'send' button in this case) is the best practice. – GameAlchemist Feb 15 '14 at 11:57

5 Answers5

17

With null-safe Dart, factory constructors are no longer permitted to return null.

Existing factory constructors that return null are expected to be replaced with static methods when migrated.

jamesdlin
  • 81,374
  • 13
  • 159
  • 204
6

Returning null value as result from a factory is acceptable because the builtin factory feature in Dart Factory software concept does not have any null-restriction.

On the other hand I can rephrase your question "Is it acceptable to return null from a equality operator"

bool operator ==(other) {
  return null;
}

This is also acceptable because there is no such restriction that this operator cannot return the null value.

But there is another question? Why do it and how to avoid it?

factory EmailAddress(String input) {
  return _regex.hasMatch(input) ? new EmailAddress._internal(input) :
    throw "something went wrong";
}

P.S.

My personal opinion that returning null from factory in Dart is a bad practice because factories in Dart are very difficult to distinguish from constructors.

From the outside they looks like constructors with the difference that they are more powerful because can construct different kinds of objects.

And they also have their restrictions but this is another story.

Zoe
  • 27,060
  • 21
  • 118
  • 148
mezoni
  • 10,684
  • 4
  • 32
  • 54
  • I agree with your opinion in this matter. I'm considering something like `var emailAddress = new EmailAddress.fromString(input);`. I think this is a bit more clear that this is in fact a factory constructor and the return value can be null. – Andrew Feb 16 '14 at 15:15
  • 1
    Even for named constructors, I would consider this an anti-pattern. When I call a constructor with `new`, I don't expect `null`. – Ganymede Feb 16 '14 at 20:20
  • @Ganymede I'm curious: now that `new` is optional (and [is discouraged](https://www.dartlang.org/guides/language/effective-dart/usage#dont-use-new)), does that change your opinion at all? Even for named factory constructors? Since those aren't distinguishable from `static` methods at callsites, do you also disapprove of having `static` methods return `null`? – jamesdlin Feb 28 '19 at 23:19
  • 3
    This is not true anymore, with sound null safety a factory cannot anymore return null since dart 2.12 – gsouf May 20 '21 at 20:15
4

I'm going to dissent from other answers: at least for named factory constructors, I don't see anything wrong with returning null.

The main differences between a factory constructor and a static method are that a factory constructor can be used with new and can be the unnamed, default constructor. Using new is now discouraged, so a named factory constructor in most cases is not distinguishable from a static method invocation at a callsite.

I don't see anything wrong with a static method returning null, and therefore I don't see anything wrong with a named factory constructor returning null either.

If the factory constructor is unnamed, then I would agree that returning null is likely to be unexpected by the caller and probably should be avoided.

That said, in such cases, there isn't much reason to use a factory constructor over a static method, and a static method is clearer.

Update

Using a static method to return null is the recommendation for code migrated to null-safety. See my new answer.

jamesdlin
  • 81,374
  • 13
  • 159
  • 204
3

It's bad practice. When someone calls a constructor they expect a non-null value.

For your case I might do validation in a static method:

class EmailAddress {
  final String _value;
  static final RegExp _regex = new RegExp(r"...");

  static bool isValid(String email) => _regex.hasMatch(email);

  EmailAddress(this._value) {
    if (!isValid(_value)) throw "Invalid email: $_value";
  }
}

Now you get code reuse and nice semantics. For instance:

querySelector("#sendButton").disabled = !EmailAddress.isValid(userEmail);
Ganymede
  • 3,345
  • 1
  • 22
  • 17
2

Please don't do this. As a user of a constructor I expect to receive an instance of the constructor's class. It's ok to return a pre-existing instance or an instance of a subtype in Dart, but don't return null.

I'd recommend one of two options to do what you want here:

  1. throw an exception on invalid inputs. This way at least the error is early, rather than later if you've stored the null somewhere.

  2. Use a static method instead of a constructor. Static methods can return null and not be as confusing.

  3. Provide a fallback path, such as int.parse does. You can accept a callback that will be called on an error.

I prefer 1 or 3 myself. I'd like to know explicitly when something isn't valid.

Justin Fagnani
  • 10,483
  • 2
  • 27
  • 37