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

Custom types with defined implicit cast to bool are unsupported by BoolToVisibility converter #53

Closed
rstroilov opened this issue Feb 3, 2018 · 6 comments
Assignees
Milestone

Comments

@rstroilov
Copy link

I faced in issue in my project. When using a type which has defined implicit cast to bool in a property, binding visibility to that property doesn't work and conversion fails with following message:
System.InvalidCastException: Specified cast is not valid.

It happens because of explicit cast is called on value in BoolToVisibilityConverter.Convert:
if ((bool)value) return Visibility.Visible;

this test reproduces issue:

        [Test]
        public void DoesNotFailConvertingWhenValueHasImplicitConversionToBool()
        {
            Assert.DoesNotThrow(() =>
                new BoolToVisibilityConverter()
                .Convert(new CastableToBoolean(true), typeof(Visibility), null, CultureInfo.CurrentCulture));
        }

        private sealed class CastableToBoolean
        {
            private bool value;

            public CastableToBoolean(bool value = false)
            {
                this.value = value;
            }

            public static implicit operator bool(CastableToBoolean obj)
            {
                return obj.value;
            }
        }
@rstroilov rstroilov changed the title Unable to convert to visibility custom type with defined implicit cast to bool Custom types with defined implicit cast to bool are unsupported by BoolToVisibility converter Feb 3, 2018
rstroilov added a commit to rstroilov/CalcBinding that referenced this issue Feb 3, 2018
…it cast to bool are unsupported by BoolToVisibility converter Alex141#53
@Alex141
Copy link
Owner

Alex141 commented Feb 4, 2018

Hi,
I agree with problem, but disagree with solution. I think it can be made shorter and more clear. How about using DLR?

     public object Convert(object value, Type targetType, object parameter, CultureInfo culture)
     {
       var boolValue = (value is bool) ? (bool)value : (bool)(dynamic)value;
       ...
     }

If we remove cast to bool after dynamic cast, we handle implicit operators. If we leave bool cast, we can handle both explicit and implicit operators. What do you say?

@rstroilov
Copy link
Author

Hi Alex,

your solution looks nice and beautiful, much better than my, but, unfortunately, it doesn't work for case specified.

... : (bool)(dynamic)value will cause same InvalidCastException
... : (dynamic)value will cause RuntimeBinderException saying that there is no implicit cast between object and bool.

@Alex141
Copy link
Owner

Alex141 commented Feb 8, 2018

Hi Roman,

Ok, I compared yours and my examples. The difference is only in access modifier of CastableToBoolean. I use 'public class' and you use 'private class'. Because of privacy of the class dlr can't acess to implicit operator. Changing access modifier on 'public' solves this problem.

Hmm, because the sample class models the class being used as the type of the source property, it must be public, as well as the property itself, otherwise the binding will simply not work, this is a general requirement for all properties (https://docs.microsoft.com/en-us/dotnet/framework/wpf/data/binding-sources-overview#other-characteristics).

So the solution of this problem is in changing the access modifier of convertable class to public.

@rstroilov
Copy link
Author

rstroilov commented Feb 13, 2018

I was wondered and checked - yes, you are right. It looks a bit confusing, that access modifier is so important here, but it really works as it should.

@Alex141
Copy link
Owner

Alex141 commented Feb 13, 2018

Ok, I glad that we solved this problem by our efforts :) I will commit the changes and release a new version tomorrow

@Alex141 Alex141 self-assigned this Feb 13, 2018
@Alex141 Alex141 added this to the 2.3.0.1 milestone Feb 13, 2018
Alex141 added a commit that referenced this issue Feb 14, 2018
@Alex141
Copy link
Owner

Alex141 commented Feb 14, 2018

Version 2.3.0.1 with this fix has been released

@Alex141 Alex141 closed this as completed Feb 14, 2018
Alex141 added a commit that referenced this issue Feb 22, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants