1

I'm using WPF converter and wondered in terms of performance what would be better in the following example, to use class members or local variables ?

    public object Convert(object value, Type targetType, object parameter,System.Globalization.CultureInfo culture)
    {
        if ((int)value == 1)
            return (Color)ColorConverter.ConvertFromString("#FCD44E");

        return (Color)ColorConverter.ConvertFromString("#FCD44E");
    }

or :

    Color _color1 = (Color)ColorConverter.ConvertFromString("#FCD44E");
    Color _color2 = (Color)ColorConverter.ConvertFromString("#FCD666");

    public object Convert(object value, Type targetType, object parameter, System.Globalization.CultureInfo culture)
    {
        if ((int)value == 1)
            return _color1;

        return _color2;
    }
Erez
  • 6,405
  • 14
  • 70
  • 124

3 Answers3

6

The most performant would be to use private static readonly as follows

private static readonly Color Color1 = (Color)ColorConverter.ConvertFromString("#FCD44E");
private static readonly Color Color2 = (Color)ColorConverter.ConvertFromString("#FCD666");

public object Convert(object value, Type targetType, object parameter, System.Globalization.CultureInfo culture)
{
    if ((int)value == 1)
        return Color1;

    return Color2;
}

See this answer for good discussion: Method can be made static, but should it?

Community
  • 1
  • 1
drstevens
  • 2,903
  • 1
  • 21
  • 30
5

While performance-wise the only relevant thing is to not do the conversion in every call to the Convert-method (as has been shown explicitly in the other answers), i would never write such a hard-coded converter in the first place, if you can parameterize something, do not hesitate to do so, e.g.:

public class OnValueToColorConverter : IValueConverter
{
    public int Value { get; set; }
    public Color OnValueColor { get; set; }
    public Color OffValueColor { get; set; }

    public object Convert(object value, Type targetType, object parameter, System.Globalization.CultureInfo culture)
    {
        return (int)value == Value ? OnValueColor : OffValueColor;
    }

    public object ConvertBack(object value, Type targetType, object parameter, System.Globalization.CultureInfo culture)
    {
        throw new NotSupportedException();
    }
}
<vc:OnValueToColorConverter Value="1"
                            OnValueColor="#FCD44E" 
                            OffValueColor="#FCD666" />

(For this sort of thing one would normally not use a converter by the way but a Style with a default setter and a DataTrigger for the value on which it should change.)

H.B.
  • 166,899
  • 29
  • 327
  • 400
  • +1 for parameterization. I don't know WPF at all but I do know it would probably be better to leave the color definitions to the xml. This is likely to be less performant than the `private static readonly` field. I would imagine the difference is unlikely to be noticed though. The most performant solution to a problem isn't always the best. It's unfortunate that properties opposed to ctor parameters are needed here though. – drstevens Sep 08 '11 at 12:54
  • @drstevens: In most scenarios this will have about the same performance as one usually only creates one instance of a converter as a resource which is then references whereever needed, so the values are also only parsed once (difference here being that an additional int is parsed). The fact that those are properties is not much of an issue either as the converters are usually not accessed in code or even inaccessible in certain patterns so the values should not be changed (if that was the corcern; one could implement some measures to ensure this of course if one is paranoid) – H.B. Sep 08 '11 at 13:16
  • :) the comment regarding properties is somewhat editorial and I probably should have kept it to myself. After working in a heavily multi-threaded environment and seeing what patterns are heavily used in other languages, I have a love-hate relationship with properties in C#. They are very enabling and often cause us to create mutable types when an immutable type is acceptable and preferable. I've come to consider property use instead of ctor params (unless absolutely necessary) to be a code smell. – drstevens Sep 09 '11 at 00:31
1

The second option, but use static readonly fields if those colors are always constant. That way you're doing the work once, not every time you create a converter.

Tim Rogers
  • 21,297
  • 6
  • 52
  • 68