Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

If you have a single property that is relevant across a full project, but you should logically have only a single representation of, how would you represent this? In my case, I am developing a simple drawing app where the user can select a colour. This colour should then be available to all the pen/shape classes in the project.

Would I simple just have a static class with a static property of Color CurrentColor (C#), that each client class could then call, or is there a more effective way?

share|improve this question
1  
Is colour a value type or reference type? –  Oded Dec 24 '11 at 10:15
    
Are the pen/shape all forced to have the same color, or is it just a default value? –  SRKX Dec 24 '11 at 10:25
    
@Oded Color is a struct, so I would say that it's a value type. @ SRKX, it would be the same color for each pen/shape. It's a very simple app aimed towards children for use with the Kinect. So it's only a simple set of colours and a small amount of pens/shapes. –  Darren Young Dec 24 '11 at 10:42
add comment

3 Answers

up vote 7 down vote accepted

Both your assumptions ("the drawing color is needed everywhere", and "there should only ever be a single instance of the current drawing color") are false. Let me elaborate:

  • A large part of your codebase is not going to be affected by the current drawing color: think about things like the UI (the parts that are not directly related to selecting the drawing color), image file format handling (loading / saving / converting), lower-level routines like showing file-open / file-save dialogs, etc. etc.
  • You may want to reuse some drawing-related functions in a different context; even though they have a notion of "current drawing color", you will run into problems soon if the current drawing color is global. Suppose you have a routine that draws a rectangle onto an image; now you want to reuse it to implement an effect filter that also needs to draw rectangles. If your current drawing color is global, you need to store it in a temporary variable, change it, draw your rectangle, and then reset it to the stored value. This is cumbersome and brittle, especially when drawing operations can be asynchronous. Another example would be if you need to handle multiple images (something MDI-ish, say), and you want to keep track of the current drawing color for each image, independently. If your current drawing color is global, you'll be cursing and swearing a lot.

Personally, I'd put all related state variables (current drawing color, current background color, current pattern, etc.) into a DrawingState class (with the usual encapsulation in place) and explicitly pass that to each drawing operation. This way, you reduce the amount of coupling and the number of potential changes to the state, and you increase the unit-testability of the drawing routines. In other words, your code will be cleaner, easier to verify, easier to debug, and easier to modify.

share|improve this answer
add comment

First of all, it is important to differentiate between the light headed remark "I need it everywhere in my project" and "I need it in all X classes in my project", because the first leads to static getters and singletons, and the latter to proper dependency injection.

Obviously Singletons are an option, but since they pretty much create global variables, you're going to want to prefer explicit atomic dependencies.

So you don't want to tie your whole project to one property (even if it currently seems like a good choice). You want to couple only what requires coupling, by definition.

In your case, one option I'd suggest is to create a ColorSelection class, and a Toolbox class (or something along those lines) that passes the current ColorSelection to a just-selected tool. In other words, the tool's Click handler (in the toolbox) would set the tool's SelectedColor property to the instance defined (and bound) by the toolbox.

In code (very inaccurate, but just so you get the point):

class ToolBox {
  ColorSelection SelectedColors {get; set;} // This could be bound to the view

  Tool Pen {get; set;}

  public event ToolEventHandler ToolSelected; // handled by the method below

  void ToolSelectedHandler(object sender, ToolEventArgs e) {
    e.Tool.SelectedColors = SelectedColors;
  }
share|improve this answer
    
That's a good idea, thanks will have a look into this. –  Darren Young Dec 24 '11 at 10:42
add comment

There exist very close to absolute zero situations where a decent design may legitimately require the use of a global variable. Global variables are as bad as goto, never use either of those.

Your simple drawing app should have a class representing the "currently selected settings". A reference to this class should be passed at construction time to every single class that may need it. So, for example, the first parameter of the constructor of your pen class should accept a reference to the "currently selected settings" class, so that the pen can fetch the currently selected color from it, or whatever else it may need, like pen thickness, opacity, solidity, step, etc.

The "currently selected settings" class should also have a "Changed" event, to which any currently active tool subscribes, so that if the user changes, say, the current color, the pen (or whatever tool happens to be selected) can take notice and re-fetch the color from it.

share|improve this answer
    
Notice that this way you're tightly coupling every tool with all the settings. Consider editing, since the event-aided idea is also a useful answer. (If you do edit, I'll remember to remove this comment) –  Yam Marcovic Dec 24 '11 at 12:18
    
@YamMarcovic You are right, I thought of this, but the programmer has to choose between either this kind of coupling or having to pass to every tool individual references to every single setting that is pertinent to the tool, and then every setting must have its own event, etc. I think this is a reasonable trade off, especially considering the fact that the coupling is not exactly tight: it is only a potential kind of coupling, meaning that the tool has access to settings that it might not need, but that does not mean that it needs to use those settings, or to know anything about them. –  Mike Nakis Dec 24 '11 at 12:26
    
It is probably a matter of opinion, but in my CoC, if there's an unnecessary dependency, I remove it and then try to find the best possible design (which is pretty much about avoiding duplication). Instead of every setting having its own event, for example, you could have the tool not sign up for any event, but only depend on things it needs (without regard for any general "set of settings"), but have the mediator between the tool and the settings define rules for updating tool properties based on changes in the settings. Would that be acceptable in your opinion? –  Yam Marcovic Dec 24 '11 at 13:01
    
Sounds promising, but then again, part of being pragmatic is also trying to keep things simple, even if that will be at the expense of perfection. Figuring out and implementing the absolute best way to do things tends to result in overengineering. –  Mike Nakis Dec 24 '11 at 13:04
    
That's only because what often happens is that after a lot of thinking, a really bad and complicated way emerges. :) About keeping it simple, the question is what we aim for - keeping the initial development simple, or keeping future changes simple? If it's the first, then we might be tempted to create a lot of coupling. If it's the latter, then I tend to side by my solution. You can check out my full opinion here. –  Yam Marcovic Dec 24 '11 at 13:11
show 2 more comments

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.