Add support for reading boolean/short/float arrays #197
Conversation
| public final static int SER_INT_ARRAY = 5; | ||
| public final static int SER_LONG_ARRAY = 6; | ||
| public final static int SER_BOOLEAN_ARRAY = 7; | ||
| public final static int SER_SHORT_ARRAY = 38; |
There was a problem hiding this comment.
... which gets tricky wrt whether to renumber entries or move these to later.
I think renumbering is needed, so move SER_TREE_NODE etc up by number of added entries needed.
There was a problem hiding this comment.
Yes, I saw that, and had the same thought, but I figured you'd know which of the two options (renumbering all later constants, or moving the other arrays to the end, which is ugly) would make the most sense.
|
@lukehutch Did you review it yourself? Or run tests? (although TBH Sonatype Snapshot repo has issues so perhaps it is difficult). Looks like unit tests fail. Overall makes sense, GenAI coding agents are pretty good at following patterns. But there are some aspects that need changing; adding comments. |
| p.nextToken(); | ||
| } | ||
|
|
||
| java.util.List<Long> values = new java.util.ArrayList<>(); |
There was a problem hiding this comment.
Very inefficient, no. Have a look at how int[] is handled.
| case JsonTokenId.ID_END_ARRAY: | ||
| break main_loop; | ||
| default: | ||
| throw new JSONObjectException("Failed to bind `long` element if `long[]` from value: "+ |
There was a problem hiding this comment.
Funny. Copied typo from int[] case :)
("if" -> "of")
(will fix separately)
| p.nextToken(); | ||
| } | ||
|
|
||
| java.util.List<Boolean> values = new java.util.ArrayList<>(); |
There was a problem hiding this comment.
Same as above, need to build an array to avoid all the boxing, unboxing.
| _generator.writeFieldName(fieldName); | ||
| writeDoubleArrayValue(v); | ||
| } | ||
|
|
|
I couldn't run the tests, and yes, this is going to be imperfect, but I figured I'd submit it as a starting point to save you a lot of work. Thanks! |
|
By the way you could also continue to iterate with Claude Code -- probably the issues you found could be fixed with a couple more prompts consisting of one or two sentences. |
My experience with Wind Surfer was... not great so I am bit of GenAI skeptic. While tools are impressive in their own right, the one and only piece of code in Jackson that I auto-generated, turned out to be the first bug reported for 2.19.0.... :-D |
|
Windsurf is pretty bad. Claude Code is almost magical, especially for tasks like this that really just require pattern matching, as long as the codebase is not enormous (in which case you run into context window size issues, where it will forget state each time it has to compact the context). |
|
@lukehutch were you able to use Claude code to look at current failures? Since we are at public repository, I suppose it should be able to fetch CI failures as well? |
|
I didn't have it look at test results or anything like that. I had it identify missing support for arrays of any of the 7 primitive types. It correctly identified that arrays of char are treated as strings, and some other special case handling. |
cowtowncoder
changed the title
cowtowncoder
changed the title
Proposed fix for #196
Created with Claude Code, so should be double-checked :-)