If you want to clean up a bit the method, then I think that you should try another, more object oriented way of writing the method (currently the method is written procedurally).
Implementation 1
First lets create few interfaces, which will represent the Limit and the Offset:
public interface Offset {
public String getOffset();
}
public interface Limit {
public String getLimit();
}
Next we will need an implementations of this interfaces (I supposed that you are using MySQL as database, since its shorter syntax for limitation and offset is the same):
public MySQLOffset implements Offset {
private final int offset;
public MySQLOffset(int offset) {
this.offset = offset;
}
@Override
public String getOffset() {
if (offset < 0) {
throw IllegalArgumentException("Offset couldn't be a negative number.")
}
return offset + ",";
}
}
public MySQLLimit implements Limit {
private final MySQLOffset offset;
private final int limit;
public MySQLLimit(MySQLOffset offset, int limit) {
this.offset = offset;
this.limit = limit;
}
@Override
public String getLimit() {
if (limit <= 0) {
throw IllegalArgumentException("Limit should be a strictly positive number.");
}
return String.format("LIMIT %s %s", offset.getOffset(), limit);
}
}
Finally we will need one more interface, which will serve us as a glue between this two interfaces (I called it Pagination, since most of the times, offset and limit is used in combination for pagination of some API):
public interface Pagination {
public String getPagination();
}
public MySQLPagination {
private final MySQLLimit limit;
private final MySQLOffset offset;
public MySQLPagination(MySQLLimit limit) {
this.limit = limit;
this.offset = offset;
}
public String getPagination() {
return limit.getLimit();
}
}
Now, suppose that we have some data access object called PersonDAO, which should retrieve all the persons in a given range (bounded by limit and offset), then the code would look like that (I`ve change the name of getPersons method to retrievePersons, since we actually retrieves the person entries from the database, rather than getting them from some POJO):
.. code ommited ..
PersonDAO.retrievePersons(new MySQLPagination(new MySQLLimit(new MySQLOffset(offset), limit)));
.. code ommited ..
public List<Person> retrievePersons(Pagination pagination) {
return jdbcTemplate.query("SELECT * FROM `Person` " + pagination.getPagination());
}
Implementation 2
Suppose that you want to you use the long syntax of limitation and offset. Then the code will look like this:
public interface Offset {
public String getOffset();
}
public interface Limit {
public String getLimit();
}
public MySQLOffset implements Offset {
private final int offset;
public MySQLOffset(int offset) {
this.offset = offset;
}
@Override
public String getOffset() {
if (offset < 0) {
throw IllegalArgumentException("Offset couldn't be a negative number.")
}
return " OFFSET " + offset;
}
}
public MySQLLimit implements Limit {
private final int limit;
public MySQLLimit(int limit) {
this.limit = limit;
}
@Override
public String getLimit() {
if (limit <= 0) {
throw IllegalArgumentException("Limit should be a strictly positive number.");
}
return "LIMIT " + limit;
}
}
public interface Pagination {
public String getPagination();
}
public MySQLPagination {
private final MySQLLimit limit;
private final MySQLOffset offset;
public MySQLPagination(MySQLLimit limit, MySQLOffset offset) {
this.limit = limit;
this.offset = offset;
}
public String getPagination() {
return limit.getLimit() + offset.getOffset();
}
}
.. code ommited ..
PersonDAO.retrievePersons(new MySQLPagination(new MySQLLimit(limit), new MySQLOffset(offset)));
.. code ommited ..
public List<Person> retrievePersons(Pagination pagination) {
return jdbcTemplate.query("SELECT * FROM `Person` " + pagination.getPagination());
}
If you want you could also move the creation of the Pagination object into some factory method like this one:
public Pagination createPagination(int limit, int offset) {
return new MySQLPagination(new MySQLLimit(limit), new MySQLOffset(offset))
}
Which also comes with some benefits, like more testable code etc.
Benefits of this implementation:
- Implemented in OOP way
- The validation of the input parameters is moved into the relevant classes, so the single responsibility principle is respected.
- Easier to change the implementation
- Easier for testing (suppose that you use one database for production and one for testing and they have different syntaxis for the limitation and the offset, then you should only mock the createPagination method and everything will work)
- There is no if/else (or ternary operator) clauses in the getPersons method
- Also now the getPersons method doesn't know how the limit and the offset of the query is constructed, which is the right way, since it`s work is to retrieve the entries, not to know how to construct them (which means little responsibilities for getPersons method)