Is there a heuristic to determine whether a method or field belongs in a class?

Question

Is there a good rule of thumb or test I can perform to determine whether a method or field belongs in a class? How can identify when a member does not belong?

I find that my single biggest stumbling block in object-oriented design is trying to figure out what goes where. It seems like there are far too many cases where the answer is: "it could go here or there."

Here's a quick example of the type of thing I'm struggling with:

Public Class ITDepartment

    Private _sysadmins As List(Of Employee)
    Private _developers As List(Of Employee)

    // properties, public stuff...

    Private Sub AddSkillToGroup(ByVal emps As List(Of Employee), ByVal skill As Skill)
        For Each e As Employee In emps
            e.AddSkill(skill)
        Next
    End Sub

End Class

The ITDepartment object manages the 2 groups of Employees...but should it know that Employees have skills? Should a method like AddSkillToGroup be relocated?

EDIT:

It seems that the consensus thus far is that the ITDepartment should not know about the employees' skills. I'll play Devil's advocate a bit to illustrate where my confusion comes into play.

The ITDepartment is composed of two Employee collections. Shouldn't it be able to delegate to those collection items? The AddSkill method still belongs to the Employee class. The ITDepartment is simply instructing its group of employees to add a skill to each of its members.

Answer 1

Look at the SOLID principles. Those will give you guidance on where a method belongs.

---

Edit

"Shouldn't [ITDepartment] be able to delegate to those collection items?"

"[is it] okay for the ITDepartment class to "reach through" and delegate to the Employee class via the List(Of Employee) that it is composed of (like its doing when it calls e.AddSkill above). "

Yes.

Delegation is how OO programming works. You delegate the details to the Single Responsible Class. You Delegate the implementation so you can depend on abstractions not implementations.

BTW, AddSkillToGroup is private, which is confusing. It does not conceal an implementation detail that stands any chance of changing. There's no reason for this to be private. [private is often over-used and used inappropriately. Very, very little should be private; and it should only be declared private when absolutely necessary.]

Since the implementation has been delegated to Employee, AddSkillToGroup is not an implementation detail of this class.

Answer 2

I'd be inclined to make List(Of Employee) into its own class at this point, so that it can have its own method AddSkill().

I guess you decide by code smells. In particular overly long argument lists; reaching inside other objects. You might also try it and see if you can make more things private than before.

Look out for methods, or collections of methods & members that form a coherent sub-group within a class - they are ripe for relocation into their own class.