Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Strategies for passing colors around (avoiding ref?)

I'm refactoring code in an aging windows application, and I've come across a pattern of sorts, that I'm not sure I like: A class has got global color variables, like the following:

private Color myForegroundColor = Color.Azure;
private Color myBackgroundColor = Color.Empty;
// ...etc. 

There are a bunch of these, and they are being passed around by ref to methods responsible for setting up certain parts of the UI.

I gather that a Color is a struct, and that each color is therefore passed by ref in order to avoid creating new copies of it each time a method is called. IE something like:

// Avoid creating a copy of myForgroundColor inside SetUpButton():
MyHelperClass.SetUpButton(ref myForegroundColor); 

I can't help feeling that this usage of ref throughout this class and related classes is bad. It feels like a "code smell", though I can't really put my finger on why.

I've seen a couple of posts on similar issues, with recommendations like "use a class containing colors, which is then passed as a value type", but it is not entirely clear how it would be best to do this.

What I would like to do is create something similar to the following:

public class ColorContainer
{
    public UiSettingsContainer()
    {
        MyColor = Color.Black;
        MyNextColor = Color.Blue;
        // ..etc...
    }

    public Color MyColor { get; private set; }
    // ...etc....
}

This would let me retain control of the colors, but the implications on memory are a little unclear to me; if I created an instance of this class and passed that around to the methods needing info about the contained colors, would not a copy of a color (with it being a struct) be created as soon as an implementing method makes use of it?

Am I correct in assuming that this code would create a new copy, and thus be less effective...

// Assumption: This creates a new copy of color in memory.
public void SetSomeColor(Color col){ 
    someComponent.color = col; 
}

// Calling it:
SetSomeColor(myColorContainerInstance.MyColor);

... than this code, which would only make use of the existing struct?:

// Question: Does this avoid creating a new copy of MyColor in memory?
public void SetSomeColor(ColorContainer container){ 
    someComponent.color = container.MyColor; 
}

// Calling it:
SetSomeColor(myColorContainerInstance);

I'm currently leaning toward a solution similar to the following, in which i gather the colors in a separate class and reorganize the code a bit, but keep using ref. In this case, however, MyColor will have to be a public field in ColorContainer, which means I will have less control over who can set it's value:

// Assumption: This creates a new copy of color in memory.
public void SetSomeColor(ref Color col){ 
    someComponent.color = col; 
}

// Calling it:
SetSomeColor(ref myColorContainerInstance.MyColor);

Is this a good solution, or are there better strategies to handle resources like this?

like image 976
Kjartan Avatar asked Jan 03 '13 10:01

Kjartan


3 Answers

This whole thing smells like premature optimization, parts 3 and 4 of the link specifically, so...

Another solution would be to just remove the refs, and copy the Color struct whenever is needed. The struct itself is not too big (4 byte members and 4 bool members), and unless you are calling the code that changes color several million times per second, the time and memory required is not an issue.

like image 123
SWeko Avatar answered Oct 16 '22 02:10

SWeko


The "slots" analogy has long been one of my favorites for this type of thing. Each method parameter (consider the right hand of assignments as parameters as well) is a slot. Each slot must be filled with something of the correct size and "shape" (type) in order for a method to be called (or an assignment to be processed).

In the case that your method requires a ref Color you're filling the slot with whatever size pointer to a Color struct in memory. Of course I don't mean the C style pointer but it's still the same sort of thing - it's a number that indicates the location of the resource you intend to use even if it's not represented in code as such. In the case of Color (without ref) you're filling it with the Color struct itself.

Depending on the platform you've compiled for, that value that you're passing around (for by ref passing of Color) will either be 32 or 64 bits in length. The color struct (System.Windows.Media.Color) itself being only 32 bits in length (or 64 bits in length if you're using System.Drawing.Color) makes this an unattractive proposition - making the average case scenario the exact same (in terms of the number of copies of the pointers and sizes of the things loaded onto the stack) as passing the struct by value - better only in the 64 bit struct/32 bit platform pairing and worse only in the 32 bit struct/64 bit platform pairing. The actual value of the struct will still be copied into its target slot even after this attempt to just have it use the same instance.

Now, bundling colors together in a class (where by ref passing is default) changes this picture a bit. If your class contains say 3 colors, you've got 96 bits (or 192 bits) of color data contained in it, passing around a maximum of 64 bits of information to find the location of the correct "package" for that information in memory. The colors will still be copied into their target slots even after the packaging; but now we've added overhead from having to either ldfld (load field)/call (call a pre-resolved method - property access) + ldfld/callvirt (call a runtime resolved method - property access) + ldfld to actually get the value. From a performance viewpoint this doesn't really help you at all, unless you intend to pass tons of data and then not use it.

The long story short - unless there's some logical grouping of color information you're trying to achieve, don't bother. Value types on the stack are cleaned up immediately when the stack frame is popped so unless your program is running at ~8 bytes short of your system's total memory you're really gaining nothing with the by ref approach. The wrapper class for a collection of colors would likely make the code cleaner/better factored but not more performant.

like image 20
mlorbetske Avatar answered Oct 16 '22 02:10

mlorbetske


Passing structs by ref is generally a good thing unless either (1) one wants pass-by value semantics, or (2) the struct is small and one can live with pass-by-value semantics.

If one will frequently be wanting to around some variables (which might be structs themselves) as a group, it may be helpful to declare a transparent struct type to hold them, and then pass that by ref.

Note that passing a struct by ref has essentially the same cost as passing a class reference by value. Writing a class to hold the struct, purely so that one can avoid using ref parameters, is not apt to be a performance win. In some cases it may be useful to have a type

class MutableHolder<T>
{ public T Value; }

which could then apply reference semantics to any struct type, but I would only suggest doing that if one needs to persist a reference outside the current scope. In cases where ref will suffice, one should use ref.

like image 24
supercat Avatar answered Oct 16 '22 01:10

supercat