[JBoss JIRA] (FORGE-2230) CoordinateBuilder should set null for empty parts of G:A:P:C:V
by Lincoln Baxter III (JIRA)
[ https://issues.jboss.org/browse/FORGE-2230?page=com.atlassian.jira.plugin... ]
Lincoln Baxter III commented on FORGE-2230:
-------------------------------------------
It's a shame we returned null here. You need to check for empty string anyway when dealing with coordinates in most systems, so this is really adding unnecessary work.
> CoordinateBuilder should set null for empty parts of G:A:P:C:V
> --------------------------------------------------------------
>
> Key: FORGE-2230
> URL: https://issues.jboss.org/browse/FORGE-2230
> Project: Forge
> Issue Type: Enhancement
> Components: Dependencies
> Affects Versions: 2.14.0.Final
> Reporter: Ondrej Zizka
> Labels: starter
> Fix For: 2.x Future
>
>
> CoordinateBuilder, when given "G:A:::V", should set P and C to null. Having them empty strings is no good.
> Consider machine-provided lists contaning:
> org.foo:foo-bar:::4.1.2
> This should end with the same result as parsing
> org.foo:foo-bar:4.1.2
> whereas it ends up with P and C set to empty strings, and forces the user of the api to do extra check for an empty string value.
> Related:
> "Malformed coordinate. Should be groupId:artifactId:[packaging]:[classifier]:[version]");
> should read
> "Malformed coordinate. Should be groupId:artifactId:[packaging:[classifier:]][version]");
> And javadoc's
> <groupId>:<artifactId>[:<packaging>[:<classifier>]]:<version>
> should read
> <groupId>:<artifactId>[:<packaging>[:<classifier>]]:[<version>]
--
This message was sent by Atlassian JIRA
(v6.3.11#6341)
9 years, 4 months
[JBoss JIRA] (FORGE-2230) CoordinateBuilder should set null for empty parts of G:A:P:C:V
by Lincoln Baxter III (JIRA)
[ https://issues.jboss.org/browse/FORGE-2230?page=com.atlassian.jira.plugin... ]
Lincoln Baxter III edited comment on FORGE-2230 at 2/11/15 10:20 AM:
---------------------------------------------------------------------
.isEmpty() is not a significant performance improvement and is not worth replacing with returning null. The bad practice of causing NPEs is *much* more harmful than a little extra code, particularly in a place where performance really does not matter until you get to the N of millions.
was (Author: lincolnthree):
.isEmpty() is not a significant performance improvement and is NOT worth replacing with returning null. The bad practice of causing NPEs is MUCH more harmful than a little extra code, particularly in a place where performance really does not matter until you get to the N of millions.
> CoordinateBuilder should set null for empty parts of G:A:P:C:V
> --------------------------------------------------------------
>
> Key: FORGE-2230
> URL: https://issues.jboss.org/browse/FORGE-2230
> Project: Forge
> Issue Type: Enhancement
> Components: Dependencies
> Affects Versions: 2.14.0.Final
> Reporter: Ondrej Zizka
> Labels: starter
> Fix For: 2.x Future
>
>
> CoordinateBuilder, when given "G:A:::V", should set P and C to null. Having them empty strings is no good.
> Consider machine-provided lists contaning:
> org.foo:foo-bar:::4.1.2
> This should end with the same result as parsing
> org.foo:foo-bar:4.1.2
> whereas it ends up with P and C set to empty strings, and forces the user of the api to do extra check for an empty string value.
> Related:
> "Malformed coordinate. Should be groupId:artifactId:[packaging]:[classifier]:[version]");
> should read
> "Malformed coordinate. Should be groupId:artifactId:[packaging:[classifier:]][version]");
> And javadoc's
> <groupId>:<artifactId>[:<packaging>[:<classifier>]]:<version>
> should read
> <groupId>:<artifactId>[:<packaging>[:<classifier>]]:[<version>]
--
This message was sent by Atlassian JIRA
(v6.3.11#6341)
9 years, 4 months
[JBoss JIRA] (FORGE-2230) CoordinateBuilder should set null for empty parts of G:A:P:C:V
by George Gastaldi (JIRA)
[ https://issues.jboss.org/browse/FORGE-2230?page=com.atlassian.jira.plugin... ]
George Gastaldi updated FORGE-2230:
-----------------------------------
Labels: starter (was: )
> CoordinateBuilder should set null for empty parts of G:A:P:C:V
> --------------------------------------------------------------
>
> Key: FORGE-2230
> URL: https://issues.jboss.org/browse/FORGE-2230
> Project: Forge
> Issue Type: Enhancement
> Components: Dependencies
> Affects Versions: 2.14.0.Final
> Reporter: Ondrej Zizka
> Labels: starter
> Fix For: 2.x Future
>
>
> CoordinateBuilder, when given "G:A:::V", should set P and C to null. Having them empty strings is no good.
> Consider machine-provided lists contaning:
> org.foo:foo-bar:::4.1.2
> This should end with the same result as parsing
> org.foo:foo-bar:4.1.2
> whereas it ends up with P and C set to empty strings, and forces the user of the api to do extra check for an empty string value.
> Related:
> "Malformed coordinate. Should be groupId:artifactId:[packaging]:[classifier]:[version]");
> should read
> "Malformed coordinate. Should be groupId:artifactId:[packaging:[classifier:]][version]");
> And javadoc's
> <groupId>:<artifactId>[:<packaging>[:<classifier>]]:<version>
> should read
> <groupId>:<artifactId>[:<packaging>[:<classifier>]]:[<version>]
--
This message was sent by Atlassian JIRA
(v6.3.11#6341)
9 years, 4 months
[JBoss JIRA] (FORGE-2230) CoordinateBuilder should set null for empty parts of G:A:P:C:V
by George Gastaldi (JIRA)
[ https://issues.jboss.org/browse/FORGE-2230?page=com.atlassian.jira.plugin... ]
George Gastaldi updated FORGE-2230:
-----------------------------------
Fix Version/s: 2.x Future
> CoordinateBuilder should set null for empty parts of G:A:P:C:V
> --------------------------------------------------------------
>
> Key: FORGE-2230
> URL: https://issues.jboss.org/browse/FORGE-2230
> Project: Forge
> Issue Type: Enhancement
> Components: Dependencies
> Affects Versions: 2.14.0.Final
> Reporter: Ondrej Zizka
> Labels: starter
> Fix For: 2.x Future
>
>
> CoordinateBuilder, when given "G:A:::V", should set P and C to null. Having them empty strings is no good.
> Consider machine-provided lists contaning:
> org.foo:foo-bar:::4.1.2
> This should end with the same result as parsing
> org.foo:foo-bar:4.1.2
> whereas it ends up with P and C set to empty strings, and forces the user of the api to do extra check for an empty string value.
> Related:
> "Malformed coordinate. Should be groupId:artifactId:[packaging]:[classifier]:[version]");
> should read
> "Malformed coordinate. Should be groupId:artifactId:[packaging:[classifier:]][version]");
> And javadoc's
> <groupId>:<artifactId>[:<packaging>[:<classifier>]]:<version>
> should read
> <groupId>:<artifactId>[:<packaging>[:<classifier>]]:[<version>]
--
This message was sent by Atlassian JIRA
(v6.3.11#6341)
9 years, 4 months
[JBoss JIRA] (FORGE-2230) CoordinateBuilder should set null for empty parts of G:A:P:C:V
by Lincoln Baxter III (JIRA)
[ https://issues.jboss.org/browse/FORGE-2230?page=com.atlassian.jira.plugin... ]
Lincoln Baxter III commented on FORGE-2230:
-------------------------------------------
.isEmpty() is not a significant performance improvement and is NOT worth replacing with returning null. The bad practice of causing NPEs is MUCH more harmful than a little extra code, particularly in a place where performance really does not matter until you get to the N of millions.
> CoordinateBuilder should set null for empty parts of G:A:P:C:V
> --------------------------------------------------------------
>
> Key: FORGE-2230
> URL: https://issues.jboss.org/browse/FORGE-2230
> Project: Forge
> Issue Type: Enhancement
> Components: Dependencies
> Affects Versions: 2.14.0.Final
> Reporter: Ondrej Zizka
>
> CoordinateBuilder, when given "G:A:::V", should set P and C to null. Having them empty strings is no good.
> Consider machine-provided lists contaning:
> org.foo:foo-bar:::4.1.2
> This should end with the same result as parsing
> org.foo:foo-bar:4.1.2
> whereas it ends up with P and C set to empty strings, and forces the user of the api to do extra check for an empty string value.
> Related:
> "Malformed coordinate. Should be groupId:artifactId:[packaging]:[classifier]:[version]");
> should read
> "Malformed coordinate. Should be groupId:artifactId:[packaging:[classifier:]][version]");
> And javadoc's
> <groupId>:<artifactId>[:<packaging>[:<classifier>]]:<version>
> should read
> <groupId>:<artifactId>[:<packaging>[:<classifier>]]:[<version>]
--
This message was sent by Atlassian JIRA
(v6.3.11#6341)
9 years, 4 months
[JBoss JIRA] (FORGE-2230) CoordinateBuilder should set null for empty parts of G:A:P:C:V
by George Gastaldi (JIRA)
[ https://issues.jboss.org/browse/FORGE-2230?page=com.atlassian.jira.plugin... ]
George Gastaldi commented on FORGE-2230:
----------------------------------------
How much slow is it? Will it severely impact performance? I don't think this is really an issue because we'll be
1) Avoiding unnecessary NullPointerExceptions: (Eg: {{coordinate.getPackaging().matches("?ar")}});
2) Promoting better code legibility (Eg: {{packaging.isEmpty()}} is better read than {{packaging != null}})
I am still not convinced we should return null, but an empty String instead.
IMHO, statements like {{coordinate.setPackaging(null)}} should throw an {{IllegalArgumentException}} because {{null}} is not a valid packaging type (unless we want to assume JAR as the default packaging)
> CoordinateBuilder should set null for empty parts of G:A:P:C:V
> --------------------------------------------------------------
>
> Key: FORGE-2230
> URL: https://issues.jboss.org/browse/FORGE-2230
> Project: Forge
> Issue Type: Enhancement
> Components: Dependencies
> Affects Versions: 2.14.0.Final
> Reporter: Ondrej Zizka
>
> CoordinateBuilder, when given "G:A:::V", should set P and C to null. Having them empty strings is no good.
> Consider machine-provided lists contaning:
> org.foo:foo-bar:::4.1.2
> This should end with the same result as parsing
> org.foo:foo-bar:4.1.2
> whereas it ends up with P and C set to empty strings, and forces the user of the api to do extra check for an empty string value.
> Related:
> "Malformed coordinate. Should be groupId:artifactId:[packaging]:[classifier]:[version]");
> should read
> "Malformed coordinate. Should be groupId:artifactId:[packaging:[classifier:]][version]");
> And javadoc's
> <groupId>:<artifactId>[:<packaging>[:<classifier>]]:<version>
> should read
> <groupId>:<artifactId>[:<packaging>[:<classifier>]]:[<version>]
--
This message was sent by Atlassian JIRA
(v6.3.11#6341)
9 years, 4 months
[JBoss JIRA] (FORGE-2230) CoordinateBuilder should set null for empty parts of G:A:P:C:V
by Ondrej Zizka (JIRA)
[ https://issues.jboss.org/browse/FORGE-2230?page=com.atlassian.jira.plugin... ]
Ondrej Zizka edited comment on FORGE-2230 at 2/11/15 10:03 AM:
---------------------------------------------------------------
Currently, parsing only "G:A:V" returns an object with getPackaging() returning null.
I recommend returning null instead of empty strings to avoid both inconsistency and behavior change (unless behavior can change in Forge across minor versions).
That statements will have to be called anyway since anyone can do coordinate.setPackaging(null).
Also, test for a null value is just one JVM step; calling string.isEmpty() is much slower.
was (Author: ozizka):
Currently, parsing only "G:A:V" returns an object with getPackaging() returning null.
I recommend returning null instead of empty strings to avoid both inconsistency and behavior change (unless behavior can change in Forge across minor versions).
Also, test for a null value is just one JVM step; calling string.isEmpty() is much slower.
> CoordinateBuilder should set null for empty parts of G:A:P:C:V
> --------------------------------------------------------------
>
> Key: FORGE-2230
> URL: https://issues.jboss.org/browse/FORGE-2230
> Project: Forge
> Issue Type: Enhancement
> Components: Dependencies
> Affects Versions: 2.14.0.Final
> Reporter: Ondrej Zizka
>
> CoordinateBuilder, when given "G:A:::V", should set P and C to null. Having them empty strings is no good.
> Consider machine-provided lists contaning:
> org.foo:foo-bar:::4.1.2
> This should end with the same result as parsing
> org.foo:foo-bar:4.1.2
> whereas it ends up with P and C set to empty strings, and forces the user of the api to do extra check for an empty string value.
> Related:
> "Malformed coordinate. Should be groupId:artifactId:[packaging]:[classifier]:[version]");
> should read
> "Malformed coordinate. Should be groupId:artifactId:[packaging:[classifier:]][version]");
> And javadoc's
> <groupId>:<artifactId>[:<packaging>[:<classifier>]]:<version>
> should read
> <groupId>:<artifactId>[:<packaging>[:<classifier>]]:[<version>]
--
This message was sent by Atlassian JIRA
(v6.3.11#6341)
9 years, 4 months
[JBoss JIRA] (FORGE-2230) CoordinateBuilder should set null for empty parts of G:A:P:C:V
by Ondrej Zizka (JIRA)
[ https://issues.jboss.org/browse/FORGE-2230?page=com.atlassian.jira.plugin... ]
Ondrej Zizka edited comment on FORGE-2230 at 2/11/15 10:02 AM:
---------------------------------------------------------------
Currently, parsing only "G:A:V" returns an object with getPackaging() returning null.
I recommend returning null instead of empty strings to avoid both inconsistency and behavior change (unless behavior can change in Forge across minor versions).
Also, test for a null value is just one JVM step; calling string.isEmpty() is much slower.
was (Author: ozizka):
Currently, parsing only "G:A:V" returns an object with getPackaging() returning null.
I recommend returning null instead of empty strings to avoid both inconsistency and behavior change (unless behavior can change in Forge across minor versions).
> CoordinateBuilder should set null for empty parts of G:A:P:C:V
> --------------------------------------------------------------
>
> Key: FORGE-2230
> URL: https://issues.jboss.org/browse/FORGE-2230
> Project: Forge
> Issue Type: Enhancement
> Components: Dependencies
> Affects Versions: 2.14.0.Final
> Reporter: Ondrej Zizka
>
> CoordinateBuilder, when given "G:A:::V", should set P and C to null. Having them empty strings is no good.
> Consider machine-provided lists contaning:
> org.foo:foo-bar:::4.1.2
> This should end with the same result as parsing
> org.foo:foo-bar:4.1.2
> whereas it ends up with P and C set to empty strings, and forces the user of the api to do extra check for an empty string value.
> Related:
> "Malformed coordinate. Should be groupId:artifactId:[packaging]:[classifier]:[version]");
> should read
> "Malformed coordinate. Should be groupId:artifactId:[packaging:[classifier:]][version]");
> And javadoc's
> <groupId>:<artifactId>[:<packaging>[:<classifier>]]:<version>
> should read
> <groupId>:<artifactId>[:<packaging>[:<classifier>]]:[<version>]
--
This message was sent by Atlassian JIRA
(v6.3.11#6341)
9 years, 4 months
[JBoss JIRA] (FORGE-2230) CoordinateBuilder should set null for empty parts of G:A:P:C:V
by Ondrej Zizka (JIRA)
[ https://issues.jboss.org/browse/FORGE-2230?page=com.atlassian.jira.plugin... ]
Ondrej Zizka edited comment on FORGE-2230 at 2/11/15 10:01 AM:
---------------------------------------------------------------
Currently, parsing only "G:A:V" returns an object with getPackaging() returning null.
I recommend returning null instead of empty strings to avoid both inconsistency and behavior change (unless behavior can change in Forge across minor versions).
was (Author: ozizka):
Currently, parsing only "G:A:V" returns an object with getPackaging() returning null.
I recommend returning null instead of empty strings to avoid inconsistency and behavior change (unless those do not matter in Forge).
> CoordinateBuilder should set null for empty parts of G:A:P:C:V
> --------------------------------------------------------------
>
> Key: FORGE-2230
> URL: https://issues.jboss.org/browse/FORGE-2230
> Project: Forge
> Issue Type: Enhancement
> Components: Dependencies
> Affects Versions: 2.14.0.Final
> Reporter: Ondrej Zizka
>
> CoordinateBuilder, when given "G:A:::V", should set P and C to null. Having them empty strings is no good.
> Consider machine-provided lists contaning:
> org.foo:foo-bar:::4.1.2
> This should end with the same result as parsing
> org.foo:foo-bar:4.1.2
> whereas it ends up with P and C set to empty strings, and forces the user of the api to do extra check for an empty string value.
> Related:
> "Malformed coordinate. Should be groupId:artifactId:[packaging]:[classifier]:[version]");
> should read
> "Malformed coordinate. Should be groupId:artifactId:[packaging:[classifier:]][version]");
> And javadoc's
> <groupId>:<artifactId>[:<packaging>[:<classifier>]]:<version>
> should read
> <groupId>:<artifactId>[:<packaging>[:<classifier>]]:[<version>]
--
This message was sent by Atlassian JIRA
(v6.3.11#6341)
9 years, 4 months
[JBoss JIRA] (FORGE-2230) CoordinateBuilder should set null for empty parts of G:A:P:C:V
by Ondrej Zizka (JIRA)
[ https://issues.jboss.org/browse/FORGE-2230?page=com.atlassian.jira.plugin... ]
Ondrej Zizka commented on FORGE-2230:
-------------------------------------
Currently, parsing only "G:A:V" returns an object with getPackaging() returning null.
I recommend returning null instead of empty strings to avoid inconsistency and behavior change (unless those do not matter in Forge).
> CoordinateBuilder should set null for empty parts of G:A:P:C:V
> --------------------------------------------------------------
>
> Key: FORGE-2230
> URL: https://issues.jboss.org/browse/FORGE-2230
> Project: Forge
> Issue Type: Enhancement
> Components: Dependencies
> Affects Versions: 2.14.0.Final
> Reporter: Ondrej Zizka
>
> CoordinateBuilder, when given "G:A:::V", should set P and C to null. Having them empty strings is no good.
> Consider machine-provided lists contaning:
> org.foo:foo-bar:::4.1.2
> This should end with the same result as parsing
> org.foo:foo-bar:4.1.2
> whereas it ends up with P and C set to empty strings, and forces the user of the api to do extra check for an empty string value.
> Related:
> "Malformed coordinate. Should be groupId:artifactId:[packaging]:[classifier]:[version]");
> should read
> "Malformed coordinate. Should be groupId:artifactId:[packaging:[classifier:]][version]");
> And javadoc's
> <groupId>:<artifactId>[:<packaging>[:<classifier>]]:<version>
> should read
> <groupId>:<artifactId>[:<packaging>[:<classifier>]]:[<version>]
--
This message was sent by Atlassian JIRA
(v6.3.11#6341)
9 years, 4 months