René's URL Explorer Experiment

{"@context":"https://schema.org","@type":"DiscussionForumPosting","headline":"DefaultConverter has many broken cases","articleBody":"The `DefaultConverter` was not being tested directly. With 4e40e04189cafcfad4c66357cbad17727dbf6325, now it is. But there are many cases that do not work, marked with commented out tests. (Also many missing tests, but at least it's a start!)\r\n\r\nThis issue exists to document some of the known problems, and thoughts on how best to resolve them.\r\n\r\nJust to give an inkling of the problems highlighted by this issue:\r\n```diff\r\n--- a/src/main/java/org/scijava/convert/DefaultConverter.java\r\n+++ b/src/main/java/org/scijava/convert/DefaultConverter.java\r\n@@ -73,6 +73,7 @@ public class DefaultConverter extends AbstractConverter\u003cObject, Object\u003e {\r\n \r\n        @Override\r\n        public Object convert(final Object src, final Type dest) {\r\n+               if (!canConvert(src, dest)) return null;\r\n \r\n                // Handle array types, including generic array types.\r\n                final Type componentType = Types.component(dest);\r\n@@ -92,6 +93,8 @@ public class DefaultConverter extends AbstractConverter\u003cObject, Object\u003e {\r\n \r\n        @Override\r\n        public \u003cT\u003e T convert(final Object src, final Class\u003cT\u003e dest) {\r\n+               if (!canConvert(src, dest)) return null;\r\n+\r\n                // ensure type is well-behaved, rather than a primitive type\r\n                final Class\u003cT\u003e saneDest = Types.box(dest);\r\n```\r\n:point_up: This patch breaks three tests. :open_mouth: \r\n\r\nSo let's consider the following questions:\r\n\r\nWhat would you expect to happen in the following circumstances? You ask SciJava Common to convert some data you have, to a particular type:\r\n\r\n1. `\"{{0, 1}, {2, 3}}\"` \u0026rarr; `double[][]`. Do you expect: A) `[[0d, 1d], [2d, 3d]]`? B) `null`?\r\n2. `\"{{0, 1}, {curtis, 3}}\"` \u0026rarr; `Double[][]`. Do you expect: A) `[[0d, 1d], [null, 3d]]`? B) `null`?\r\n3. `\"{{0, 1}, {curtis, 3}}\"` \u0026rarr; `double[][]`. Do you expect: A) `[[0d, 1d], [NaN, 3d]]`? B) `null`?\r\n4. `\"5\"` \u0026rarr; `double[]`. Do you expect: A) `[5d]`? B) `null`?\r\n5. `\"5\"` \u0026rarr; `double[][]`. Do you expect: A) `[[5d]]`? B) `null`?\r\n6. `\"curtis\"` \u0026rarr; `double[]`. Do you expect: A) `[NaN]`? B) `null`?\r\n7. `\"curtis\"` \u0026rarr; `Double[]`. Do you expect: A) `[null]`? B) `null`?\r\n8. `\"curtis\"` \u0026rarr; `Double[][]`. Do you expect: A) `[[null]]`? B) `null`?\r\n\r\nI think the following behavior is what we should shoot for:\r\n\r\n* When converting to an array/collection type, we first go to `Object[]`, `Object[][]`, etc., of correct dimensionality.\r\n* If dimensionality of this initially converted source is less than dimensionality of target, we wrap it in extra dimensions till it's the same.\r\n* If dimensionality of the initially converted source is *more* than the dimensionality of target, return `null`.\r\n* Otherwise: we allocate the destination object of the requested type, then convert source's leaf elements one by one to destination elements.\r\n\r\nE.g. for (8) above, we would go `\"curtis\"` \u0026rarr; `new Object[][] {{\"curtis\"}}` \u0026rarr; `new Double[][] {{ convert(\"curtis\", Double.class) }}`.\r\n\r\nThe remaining question is: what to do if any/all of those leaves are not convertible?\r\n\r\nAfter some discussion between myself, @hinerm, and @gselzer, we narrowed down the following potentially reasonable behavior:\r\n\r\n1. If *all* of the leaf elements are convertible to the destination element type, then the converted object is populated. Otherwise, the *whole collection* is inconvertible, and we return `null` for the whole collection, and `false` for `canConvert`. This would answer the above list with `null` for questions 2, 3, 6, 7, 8.\r\n\r\n2. If *any* of the leaf elements are convertible to the destination element type, then the converted object is populated, filling any inconvertible elements with `null`. This would answer the above list with `null` for questions 6, 7, 8—but 2 and 3 would work.\r\n\r\n3. *Regardless* of whether any leaf elements are convertible to the destination element type, the converted object is populated, filling nulls as in (2). This would answer the above list with \"As all the way down\" i.e. no `null` results.\r\n\r\nPart of this issue is deciding which of these three behaviors would be best. Then fixing the `DefaultConverter` to work that way—in both its `canConvert` and `convert` methods—and uncommenting the disabled tests (and adjusting them as needed) to validate it.","author":{"url":"https://github.com/ctrueden","@type":"Person","name":"ctrueden"},"datePublished":"2023-02-01T21:19:56.000Z","interactionStatistic":{"@type":"InteractionCounter","interactionType":"https://schema.org/CommentAction","userInteractionCount":2},"url":"https://github.com/450/scijava-common/issues/450"}