persistence functions should return Optional instead of null

[querdenker2k]

All persistence functions should return an Optional instead of null. This is for better handling. Optional is more visible that it can be null. Furthermore if an Item is not found an ItemFoundException will be thrown (RuntimeException).

[seaside1]

Overall this commit makes sense and is a good refactoring that makes the overall code better.

• Optional instead of null, makes it more clear and not fuzzy
• Exceptions when trying to get items that might not exist
• ZonedDateTIme instead of Date (where zd is the more modern approach)

I'm a bit split with throwing exceptions around when it is expected, like:

      public static <T> T get(String itemName, Class<T> jRuleItemClass) throws JRuleItemNotFoundException {
        JRuleItem jruleItem = itemRegistry.get(itemName);
        if (jruleItem == null) {
            verifyThatItemExist(itemName);

            try {
                Constructor<T> constructor = jRuleItemClass.getDeclaredConstructor(String.class);
                constructor.setAccessible(true);
@@ -43,4 +50,17 @@ public static <T> T get(String itemName, Class<T> jRuleItemClass) {
        }
        return (T) jruleItem;
    }

    private static void verifyThatItemExist(String itemName) throws JRuleItemNotFoundException {
        try {
            ItemRegistry itemRegistry = JRuleEventHandler.get().getItemRegistry();
            if (itemRegistry == null) {
                throw new IllegalStateException(
                        String.format("Item registry is not set can't get item for name: %s", itemName));
            }
            itemRegistry.getItem(itemName);
        } catch (ItemNotFoundException e) {
            throw new JRuleItemNotFoundException(String.format("cannot find item for name: %s", itemName), e);
        }
    }

Where verifyThatItemExists will throw an exception as verification.
@seime Any comments before merge?

[seime]

Agree on the comment, we should try to avoid using exceptions for control flow as it is an anti pattern - especially where there is an easy way around.

Aside from that I agree on the PR changes. Thanks @querdenker2k

[querdenker2k]

But i think from the usage view its the easiest way to handle that an item which was requested does not exist. In my opinion this exception is not for control flow because its not catched within jrule.
Just returning null will cause consequential errors inside the user rules. An exception is more clear to the user whats the problem when looking into the logs.
The other option would be returning an optional object, this would be ok for me.
What do you think? @seime @seaside1

[seime]

I've changed my mind and vote to merge as is @seaside1

[seaside1]

Ok let's merge then. We can always change it later if we change our mind.