Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CompareStateTrigger, EqualsStateTrigger and NotEqualStateTrigger can return unexpected results #37

Open
deepea opened this issue Jul 19, 2015 · 6 comments

Comments

@deepea
Copy link

deepea commented Jul 19, 2015

CompareStateTrigger, EqualsStateTrigger and NotEqualStateTrigger, since it uses EqualsStateTrigger internally, can return unexpected results because they use == for comparison rather than object.Equals(…). The result is a reference comparison when that was probably not intended.

Fixes:

Change comparisons from using == to object.Equals(…).

CompareStateTrigger.cs
internal Comparison CompareValues()
{//if (v1 == v2) OLD METHOD
    if (object.Equals(v1, v2)) // NEW METHOD
    {
        if (Comparison == Comparison.Equal)
            return Comparison.Equal;
    }}
EqualsStateTrigger.cs
internal static bool AreValuesEqual(object value1, object value2, bool convertType)
{
    //if (value1 == value2) OLD METHOD
    if (object.Equals(value1, value2)) // NEW METHOD
    {
        return true;
    }}

To Reproduce:

The provided example shows the issue with the EqualsStateTrigger, using boolean values. CompareStateTrigger seems to do better in this particular case because it uses IComparable when possible. However, CompareStateTrigger has given me similar issues using enums. I can provide an example of the latter as proof if necessary.

MainPage.xaml
<Page
    x:Class="TestComparisons.MainPage"
    xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
    xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
    xmlns:local="using:TestComparisons"
    xmlns:d="http://schemas.microsoft.com/expression/blend/2008"
    xmlns:mc="http://schemas.openxmlformats.org/markup-compatibility/2006"
    xmlns:wst="using:WindowsStateTriggers"
    mc:Ignorable="d">

    <Page.Resources>
        <x:Boolean x:Key="TrueValue">True</x:Boolean>
        <x:Boolean x:Key="AnotherTrueValue">True</x:Boolean>
    </Page.Resources>

    <Grid Background="{ThemeResource ApplicationPageBackgroundThemeBrush}">
        <VisualStateManager.VisualStateGroups>
            <VisualStateGroup>
                <VisualState>
                    <VisualState.StateTriggers>
                        <wst:EqualsStateTrigger Value="{StaticResource TrueValue}" EqualTo="{StaticResource AnotherTrueValue}" />
                    </VisualState.StateTriggers>
                    <VisualState.Setters>
                        <Setter Target="comparisonStatus.Text" Value="The values match, as expected" />
                    </VisualState.Setters>
                </VisualState>
                <VisualState>
                    <VisualState.StateTriggers>
                        <wst:NotEqualStateTrigger Value="{StaticResource TrueValue}" NotEqualTo="{StaticResource AnotherTrueValue}" />
                    </VisualState.StateTriggers>
                    <VisualState.Setters>
                        <Setter Target="comparisonStatus.Text" Value="The values do not match! Unexpected." />
                    </VisualState.Setters>
                </VisualState>
            </VisualStateGroup>
        </VisualStateManager.VisualStateGroups>

        <TextBlock x:Name="comparisonStatus" Margin="3" />
    </Grid>
</Page>
@dotMorten
Copy link
Owner

Only types that implement IComparable are supported for this Compare trigger. If these are reference objects, I would actually expect it to be a by-reference comparison. So IMHO the current behavior is correct.
For the equals/not-equals it is using the .Equals method, but will try a quick simple reference comparison first.

@SvenLauterbach
Copy link

I don't know if this is the correct issue, but i'm facing a very strange behavior with the EqualStateTrigger. It's simply does not trigger. When i debug the trigger something odd happens:

Strange behavior EqualStateTrigger

As you can see, both values are 0 and both values are int, but the comparison is false. Therefore the trigger never fires.....

@dotMorten
Copy link
Owner

@SvenLauterbach If you continue stepping into this method, where does it go wrong?

@SvenLauterbach
Copy link

@dotMorten it goes wrong in the very first line, because the first if statement should be entered, but for some reason "value1 == value2" returns "false" (see my screenshot). Instead the execution goes into the second if statement (both values are not null) and then it goes straight to the "return false" at the bottom of the method because the third if statement is also false (both values have the same type as you can see in my screenshot). So, both values are 0, both values are int, so i think it should just enter the first if statement and return true - but it doesn't.

@SvenLauterbach
Copy link

using .Equal() does the trick.....

A good explanation why : http://blogs.msdn.com/b/csharpfaq/archive/2004/03/29/when-should-i-use-and-when-should-i-use-equals.aspx

@Viachaslau-Zinkevich
Copy link

The strings doesn't compare through ==, too. When they are considered as objects, they should be compared through Equals.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants