Bug in comparator of ConfigSource

classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

Bug in comparator of ConfigSource

Marc Schorderet
Hi guys,



Today I found an issue by adding one more configuration file for my
application. We are using version 1.8.0.



It seems that your comparator of ConfigSource only returns -1 and 1; never
0. So, when two config files have the same original, the application
crashes. We have some common configuration files that are not managed by
myself, so I cannot guarantee that the original is unique. But I can
guarantee that original is unique for each application domain, so we have
never issues with priority.



private static ConfigSource[] sortDescending(List<ConfigSource>
configSources)
{
    Collections.*sort*(configSources, new Comparator<ConfigSource>()
    {



*/**          * {@inheritDoc}          */         *@Override
        public int compare(ConfigSource configSource1, ConfigSource
configSource2)
        {
            return (configSource1.getOrdinal() >
configSource2.getOrdinal()) ? -1 : 1;
        }
    });
    return configSources.toArray(new ConfigSource[configSources.size()]);
}

private static List<ConfigSource> sortAscending(List<ConfigSource>
configSources)
{
    Collections.*sort*(configSources, new Comparator<ConfigSource>()
    {



*/**          * {@inheritDoc}          */         *@Override
        public int compare(ConfigSource configSource1, ConfigSource
configSource2)
        {
            return (configSource1.getOrdinal() >
configSource2.getOrdinal()) ? 1 : -1;
        }
    });
    return configSources;
}



When we use the Collection.sort with default TimSort algorithm, Java throws
a "Comparison method violates its general contract!" Exception. Is it not
better to use default comparator of integer in this case?



Thanks,
Marc


--
Schorderet Marc
[hidden email]
Reply | Threaded
Open this post in threaded view
|

Re: Bug in comparator of ConfigSource

Romain Manni-Bucau
Hi Marc

You are right, do you want to submit a PR or patch?

Romain


Le mar. 29 mai 2018 20:43, Marc Schorderet <[hidden email]> a
écrit :

> Hi guys,
>
>
>
> Today I found an issue by adding one more configuration file for my
> application. We are using version 1.8.0.
>
>
>
> It seems that your comparator of ConfigSource only returns -1 and 1; never
> 0. So, when two config files have the same original, the application
> crashes. We have some common configuration files that are not managed by
> myself, so I cannot guarantee that the original is unique. But I can
> guarantee that original is unique for each application domain, so we have
> never issues with priority.
>
>
>
> private static ConfigSource[] sortDescending(List<ConfigSource>
> configSources)
> {
>     Collections.*sort*(configSources, new Comparator<ConfigSource>()
>     {
>
>
>
> */**          * {@inheritDoc}          */         *@Override
>         public int compare(ConfigSource configSource1, ConfigSource
> configSource2)
>         {
>             return (configSource1.getOrdinal() >
> configSource2.getOrdinal()) ? -1 : 1;
>         }
>     });
>     return configSources.toArray(new ConfigSource[configSources.size()]);
> }
>
> private static List<ConfigSource> sortAscending(List<ConfigSource>
> configSources)
> {
>     Collections.*sort*(configSources, new Comparator<ConfigSource>()
>     {
>
>
>
> */**          * {@inheritDoc}          */         *@Override
>         public int compare(ConfigSource configSource1, ConfigSource
> configSource2)
>         {
>             return (configSource1.getOrdinal() >
> configSource2.getOrdinal()) ? 1 : -1;
>         }
>     });
>     return configSources;
> }
>
>
>
> When we use the Collection.sort with default TimSort algorithm, Java throws
> a "Comparison method violates its general contract!" Exception. Is it not
> better to use default comparator of integer in this case?
>
>
>
> Thanks,
> Marc
>
>
> --
> Schorderet Marc
> [hidden email]
>