[
https://issues.apache.org/jira/browse/SOLR-6013?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13981528#comment-13981528
]
Shalin Shekhar Mangar commented on SOLR-6013:
---------------------------------------------
Thanks Aaron. Some comments:
# DateFormatEvaluator methods such as evaluateWrapper, evaluateString,
parseMathString, resolveMapper and getDateMathParser have no business being
public but we can make them protected if you want. All of them should be marked
as experimental API.
# Evaluator.parseParams should be protected not public.
# I don't like that we are creating a method such as getVariableWrapper. These
things are not supposed to be pluggable and it should definitely not be public.
We can mark it as protected along with a javadoc warning saying that this is
experimental API. If I were writing Evaluator today, I'd just use a Callable
instead.
On an unrelated note, I wonder why we cache all available locales and
timezones. If looking up locale/timezone is expensive then it can be done
as-needed. I'll create an issue.
> Fix method visibility of Evaluator, refactor DateFormatEvaluator for
> extensibility
> ----------------------------------------------------------------------------------
>
> Key: SOLR-6013
> URL: https://issues.apache.org/jira/browse/SOLR-6013
> Project: Solr
> Issue Type: Improvement
> Components: contrib - DataImportHandler
> Affects Versions: 4.7
> Reporter: Aaron LaBella
> Fix For: 4.8
>
> Attachments: 0001-add-getters-for-datemathparser.patch,
> 0001-change-method-variable-visibility-and-refactor-for-extensibility.patch
>
> Original Estimate: 1h
> Remaining Estimate: 1h
>
> This is similar to issue 5981, the Evaluator class is declared as abstract,
> yet the parseParams method is package private? Surely this is an oversight,
> as I wouldn't expect everyone writing their own evaluators to have to deal
> with parsing the parameters.
> Similarly, I needed to refactor DateFormatEvaluator because I need to do some
> custom date math/parsing and it wasn't written in a way that I can extend it.
> Please review/apply my attached patch to the next version of Solr, ie: 4.8 or
> 4.9 if I must wait.
> Thanks!
--
This message was sent by Atlassian JIRA
(v6.2#6252)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
