alexfu / SQLiteQueryBuilder

A library that provides a simple API for building SQLite query statements in Java.
MIT License
70 stars 21 forks source link

Add argument checks and fail early #9

Closed alexfu closed 9 years ago

alexfu commented 9 years ago

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 commented 9 years ago

@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 commented 9 years ago

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 commented 9 years ago

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 commented 9 years ago

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 commented 9 years ago

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

shadeven commented 9 years ago

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 commented 9 years ago

@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 commented 9 years ago

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

alexfu commented 9 years ago

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 commented 9 years ago

@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 commented 9 years ago

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