Skip to content
This repository has been archived by the owner on Mar 2, 2019. It is now read-only.

Add argument checks and fail early #9

Closed
alexfu opened this issue Mar 28, 2015 · 11 comments
Closed

Add argument checks and fail early #9

alexfu opened this issue Mar 28, 2015 · 11 comments

Comments

@alexfu
Copy link
Owner

alexfu commented Mar 28, 2015

In each builder, we should have some sort of argument check, like this.

Errors should be caught and thrown as early as possible. If, for example, these checks were done right before the build() function was called, it would be extremely hard to debug since the stack trace would only point to the toString()/build() line and not where the actual error happened.

@shadeven
Copy link
Contributor

@alexfu, that's what I normally do for methods exposed to an interface. We can create a separate class which is only responsible for validation or create a new interface containing a valid() method which every builder should implement.

What do you think?

@monxalo
Copy link
Contributor

monxalo commented May 19, 2015

You can use Preconditions for this.

public CreateTableBuilder column(Column column) {
  Preconditions.checkNotNull(column, "A non-null column is required.");

  definitions.add(column);
  return this;
}

@shadeven
Copy link
Contributor

Or we can use the Java Objects class instead of introducing new library.

public CreateTableBuilder column(Column column) {
  Objects.requireNonNull(column, "A non-null column is required.");
  definitions.add(column);
  return this;
}

If the validation becomes more detailed and specific, then we need to create our own.

@monxalo
Copy link
Contributor

monxalo commented May 19, 2015

Didn't know Objects had those checks. If It's just to check if an argument is null i agree.

Preconditions have other checks such as checkArgument(boolean expression) and throws an IllegalArgumentException accordingly.

It won't introduce another library, we can just include the Preconditions.java file (or just some methods) as it doesn't have any dependencies.

@monxalo
Copy link
Contributor

monxalo commented Jun 4, 2015

Just noticed that in case of using on Android the Objects class is only available >= API 19, so just a limitation there.

@shadeven
Copy link
Contributor

shadeven commented Jun 4, 2015

Oops, it is a limitation. Use other available library or check null by yourself then.
What sort of android project are you working on at the moment?

@monxalo
Copy link
Contributor

monxalo commented Jun 4, 2015

@shadeven just a personal app, ticket manager for train trips here in portugal. It only imports the tickets from pdfs that the company sends by email; notifies of the next trip with convenient information like gate, carriage and seat number.

@shadeven
Copy link
Contributor

shadeven commented Jun 4, 2015

@monxalo, sounds interesting. I want to up-skill in android development area, please let me know if you need extra hands.

@alexfu
Copy link
Owner Author

alexfu commented Jun 4, 2015

I'm in favor for doing null checks by hand or use Assert.assertNotNull. Using a third party library like Guava brings in unnecessary code in addition to depending on yet another library.

@monxalo
Copy link
Contributor

monxalo commented Jun 4, 2015

@alexfu Like i said above you don't need to add the library as a dependency, just add the file needed to the util package like u2020.

@alexfu
Copy link
Owner Author

alexfu commented Jun 4, 2015

@monxalo sorry, must of missed that. that works too.

alexfu added a commit that referenced this issue Jul 7, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants