ParameterValue.toXYZ() returns Object instead of primitive got numeric, boolean, chart

[decebals]

My goal is to simplify the parameter value read process for null or empty values.

Before this PR:

// the correct approach for test null or empty value ParameterValueidParam = routeContext.getParameter("id");
if (idParam.isNull() || idParam.isEmpty()) {
// no id parameter
} else {
longid = idParam.toLong(); // do my job
}
// the short version
```javalongid = routeContext.getParameter("id").toLong();
if (id > 0) {
// do my job
}

The problems with last snippet are:

• I cannot know if the request parameter is null or is 0 (or other default value)
• I am in trouble if the request parameter is empty (I am encountered some situations) because a "NumberFormatException" is thrown; this situation can be avoided if we change isNull() from check if (isNull()) { return defaultValue; } with a more comprehensive isNullOrEmpty()

With current PR we can write:

Longid = routeContext.getParameter("id").toLong();
// id is null if the parameter value is null or emptyif (id != null) {
// do my job
}

For my tests this PR does't not affect existing applications built with Pippo.

What do you think, brings value or not this PR?

[gitblit]

I think this will break some (all?) existing apps that use controllers with primitive types.

publicvoidgetItems(intpage, intsize, booleanincludeMetadata) {
}

In the above example we'll get ClassCastExceptions because null can't be cast to a primitive. I'd rather we think more about how to improve the API to address your need without breaking the above use-case.

[decebals]

I have the same problem in PippoSettings. I don't like (for example in UndertowServer) to use something like:

if (getSettings().getWorkerThreads() > 0) {
builder.setWorkerThreads(getSettings().getWorkerThreads());
}

I prefer this construction:

if (getSettings().getWorkerThreads() != null) {
// I am sure that "workerThreads" doesn't exist in settings 
...
}

It's a little tricky to assign a default value for a null (missing) parameter/setting. From my point of view the code is more clean with object (Integer, Boolean) instead of primitives.

I think this will break some (all?) existing apps that use controllers with primitive types.

Sure, I see this thing and it's a problem for controllers. I don't know is it's not possible to make a compromise for controllers (the idea is to use toXYZ(name, defaultValue) to be sure that I don't have a null value).

[gitblit]

I'd really prefer not to lose the simplicity of primitive values.

Your proposed syntax:

Longid = routeContext.getParameter("id").toLong();
// id is null if the parameter value is null or emptyif (id != null) {
// do my job
}

My syntax counter-proposal:

Longid = null;
if (routeContext.hasParameter("id")) { // empty value is acceptableid = routeContext.getParameter("id").toLong(); // primitive auto-boxed to object
}
// ORif (routeContext.hasParameterValue("id")) { // empty value is not acceptableid = routeContext.getParameter("id").toLong(); // primitive auto-boxed to object
}
if (id != null) {
// do my job
}

Looks like we need to fixup things like toLong(long defaultValue) to gracefully handle isEmpty instead of just isNull while still defaulting to 0.

From my point of view the code is more clean with object (Integer, Boolean) instead of primitives.

Interesting. I have the exact opposite opinion. When reviewing code a deeper understanding of runtime behavior is required when you use the object peers of primitives and you introduce more surface area for NPEs. Switching to objects also adds value sanity checks in addition to normal range checks:

if (getSettings().getWorkerThreads() != null) {
// I am sure that "workerThreads" doesn't exist in settings if (getSettings().getWorkerThreads() > 0) {
// you can't assign 0 (or fewer) worker threads
...
}
}