Defining schema code comments (was: Re: [GitHub] incubator-pirk pull request #26: [PIRK-19] Make DataSchema/QuerySchema Agnos...)

[Tim Ellison <t....@gmail.com>]

On 27/07/16 03:17, ellisonanne wrote:
> As a very small (kind of nitty) style item, I would much rather have
> setter methods for QuerySchema to set the `elementName`  and
> `filterNamesSet` rather than setting these fields via
> 
> ``` querySchema.getElementNames().addAll(elementNames); 
> querySchema.getFilteredElementNames().addAll(filterNamesSet); ```
> 
> In the original code, these were passed in as part of the constructor
> (which has now been removed in this refactor).

Sure, I can loop back and change that.

So the thinking was that rather than parse all the XML and build the
schema in a 'big bang', the code now creates an empty schema and then
populates it incrementally as it is being parsed.  I agree it is a style
thing, but IMHO incremental composition leads to less, simpler code.

I can change the parser to remember all the element names and set them
at once if you prefer.

Either way the creation of a new schema is being controlled by builders
in the o.a.pirk.schema.{data|query} packages, with a goal that they are
immutable once created [1].  So any setters would remain default
(package) scope visibility.

Do you agree that schemas should be immutable (by schema API users)?

[1] still not quite true yet today.

Regards,
Tim

Re: Defining schema code comments

[Tim Ellison <t....@gmail.com>]

On 27/07/16 11:33, Ellison Anne Williams wrote:

> Yes, I do agree that schemas are immutable once created - I can't think of
> a good/plausible reason why we would need to change a schema once it's
> created...
> 
> The incremental composition that you added is very clean -- along those
> lines, the code refactor of the Data and Query schema framework is very
> nice and clean, a good addition/improvement to the project :)

Thanks for the feedback Ellison Anne, I appreciate it.

I'm pretty much done there now but will pop back and fix any issues or
add a few minor enhancements as I try to use the schemas in my sample
programs.

Regards,
Tim

Re: Defining schema code comments (was: Re: [GitHub] incubator-pirk pull request #26: [PIRK-19] Make DataSchema/QuerySchema Agnos...)

[Ellison Anne Williams <ea...@gmail.com>]

Hi --

Yes, I do agree that schemas are immutable once created - I can't think of
a good/plausible reason why we would need to change a schema once it's
created...

The incremental composition that you added is very clean -- along those
lines, the code refactor of the Data and Query schema framework is very
nice and clean, a good addition/improvement to the project :)

On Wed, Jul 27, 2016 at 4:34 AM, Tim Ellison <t....@gmail.com> wrote:

> On 27/07/16 03:17, ellisonanne wrote:
> > As a very small (kind of nitty) style item, I would much rather have
> > setter methods for QuerySchema to set the `elementName`  and
> > `filterNamesSet` rather than setting these fields via
> >
> > ``` querySchema.getElementNames().addAll(elementNames);
> > querySchema.getFilteredElementNames().addAll(filterNamesSet); ```
> >
> > In the original code, these were passed in as part of the constructor
> > (which has now been removed in this refactor).
>
> Sure, I can loop back and change that.
>
> So the thinking was that rather than parse all the XML and build the
> schema in a 'big bang', the code now creates an empty schema and then
> populates it incrementally as it is being parsed.  I agree it is a style
> thing, but IMHO incremental composition leads to less, simpler code.
>
> I can change the parser to remember all the element names and set them
> at once if you prefer.
>
> Either way the creation of a new schema is being controlled by builders
> in the o.a.pirk.schema.{data|query} packages, with a goal that they are
> immutable once created [1].  So any setters would remain default
> (package) scope visibility.
>
> Do you agree that schemas should be immutable (by schema API users)?
>
> [1] still not quite true yet today.
>
> Regards,
> Tim
>