[
https://issues.apache.org/jira/browse/AVRO-242?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12785142#action_12785142
]
Philip Zeyliger commented on AVRO-242:
--------------------------------------
Overall looks good.
I think the code would be simpler (fewer null checks, equalProps & propsHash
wouldn't be necessary) if you didn't mind creating an extra HashMap per Schema.
That is, just start with an empty map.
bq. Iterator<String> i = schema.getFieldNames();
Do you dislike the Java 1.5 for syntax? You can put the two lines you save
towards your braces account. (<ducks>)
bq. setProp()/getProp
For a public API, I have a mild preference for setProperty() and getProperty().
I think that's the API you suggested before.
bq. schema.get(prop).getTextValue()
There's sometimes not going to be a text value. Is there a test for that? You
should, presumably, just skip those properties.
bq. static final Map<String,Type> PRIMITIVES = new HashMap<String,Type>();
Unrelated to the current issue, but since you're changing these lines... You
could write this as:
for (Type t : Type.somethingOrOther()) {
if (t.isPrimitive()) {
PRIMITIVES.add(type.name, type);
}
}
You'd need to update the Type enum to have a boolean for whether things are
primitive or not, which seems useful anyway.
It seems visually useful to put the PRIMITIVES list and the Type enum closer
together in the java file.
> add support for per-schema metadata
> -----------------------------------
>
> Key: AVRO-242
> URL: https://issues.apache.org/jira/browse/AVRO-242
> Project: Avro
> Issue Type: New Feature
> Components: java
> Reporter: Doug Cutting
> Assignee: Doug Cutting
> Fix For: 1.3.0
>
> Attachments: AVRO-242.patch
>
>
> The Avro specification permits schema json to contain other property names to
> be used besides those listed in the specification. I propose adding support
> to Java for getting and setting these.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
