CODEWITHSUNDEEP
c#static-methods
Siegeon
Siegeon

Reputation: 610

Static member access

It is likely that I am going about this all wrong, but I have a user control called CategoryControl, there can be many like it, for that reason I decided that many of its functions would be better served as static methods. I wanted to know if there is a "better" way of accessing these methods then passing an instance all over the class. The methods are public static as they will be updated by other methods. The though of making extension methods comes to mind..?

public CategoryControl(UserCategory userCategory)
{
   InitializeComponent();

   PopulateControl(userCategory, this);
}

private static void PopulateControl(UserCategory userCategory, CategoryControl instance)
{

   SetCategoryTitle(userCategory, instance);

   SetPercentCorrect(userCategory, instance);

   SetQuestionsMissed(userCategory, instance);

   SetBackgroundBar(userCategory, instance);

   SetForegroundBar(userCategory, instance);

}

Updated::

The longer story is that I have a Panel on the screen, the panel contains relevant user categories. By relevant I mean that the user has the option of changing courses thus displaying a new set of categories. A user can also change the values of a category based on their interaction with the software. So...

A panel shows the categories of a course.

I maintain a list of the active Category Controls in the panel, and the main form tells the panel when to draw a new set of categories. 

public void InitializeProgressPanel(UserCategories parentCategories)
{
   Contract.Requires(parentCategories != null, "parentCategories is null.");

   RemoveAllControlsFromList(_categoryControls);

   UserCategories sortedUserCategories = parentCategories.SortByWorst();

   int categoriesCount = parentCategories.Count();

   int spacer = (Height - (CategoryControl.Controls_Height * categoriesCount)) / categoriesCount+1;

   for (int i = 0; i < sortedUserCategories.Count; i++)
   {
      CategoryControl cc = new CategoryControl((UserCategory)sortedUserCategories[i]);

      cc.Left = 0;

      if (i == 0)
         cc.Top = spacer;
      else
         cc.Top = (Controls[i - 1].Bottom + spacer);

      Controls.Add(cc);
      _categoryControls.Add(cc);

      }
}

Upvotes: 0

Views: 218

Answers (5)

Eric Lippert
Eric Lippert

Reputation: 659984

I would certainly not make extension methods if I had a class in hand that I could extend. Remember, the purpose of extension methods is to extend types that you cannot extend yourself.

The question at hand then is, should you say:

class C 
{
    public void Foo() { ... }
}

or

class C
{
    public static void Foo(C c) { ... }
}

I would ask some questions like:

  • Is the class ever going to be subclassed? If so, should this be a virtual method?
  • Is Foo the kind of thing that an instance does to itself, or the sort of thing that it has done to it? An animal eats on its own, but an animal is fed by someone else.

UPDATE:

Some more questions I'd ask myself:

  • Are the properties and whatnot you are setting ever going to change? The less mutability you have in a class, the easier it is to test, the easier it is to reason about, and the fewer bugs you'll have. If the properties and whatnot are never going to change then do not set them in any kind of method. Set them in the constructor and then never worry about them again; they're correct.

Upvotes: 8

Jason Williams
Jason Williams

Reputation: 57892

Some questions may help...(?)

  • What benefit do you perceive in making the methods static? Converting the method to static, you are taking away the implicit passing of "this", and passing it in manually every time. How does that help? (It won't make the code any more efficient, it just means you have to pass 'instance' into every call you make, so you need to write more code)

  • Does the user category change a lot? If not, rather than passing it in for every call, would it make more sense to make it a member variable?

  • Would you really want to call all these static methods one by one to change all the different parameters of the control? Look at how the client will use this class and you may find that you can roll all of those options into one or two methods that take a bunch of parameters and apply them all in one hit. (Often if you want to change one setting, you will want to change several settings together)

Upvotes: 0

StriplingWarrior
StriplingWarrior

Reputation: 156459

Just a shot in the dark here, but I'd probably try for something more like this:

private void PopulateControl(UserCategory userCategory)
{
    CategoryTitle = GetCategoryTitle(userCategory);
    PercentCorrect = GetPercentCorrect(userCategory);
    ...
}

Upvotes: 1

rice
rice

Reputation: 1063

Seems better to have the functionality on one of the two classes involved in the interaction, rather than on some third party.

Here are two ways that spring to mind:

  1. CategoryControl could have a public function PopulateCategory(UserCategory userCat)
  2. UserCategory could have a public function PopulateFromControl(CategoryControl ctrl)

If all those operations about title and percent etc need to be separate actions, you'd just follow the model above but have separate functions for each item.

Upvotes: 1

Jason
Jason

Reputation: 3960

Why not make them instance members, and do it like this

private UserCategory _userCategory;

public CategoryControl(UserCategory userCategory)
{
   InitializeComponent();

   this._userCategory = userCategory;
   this.PopulateControl();
}

private void PopulateControl()
{
   // to see userCategory you'd do "this._userCategory"
   // to see the specific instance you could simply do "this"

   SetCategoryTitle();

   SetPercentCorrect();

   SetQuestionsMissed();

   SetBackgroundBar();

   SetForegroundBar();

}

Upvotes: 1

Related Questions