9
\$\begingroup\$

I have a class that stores the data related to all employees in a particular department. In this class, I am having a list of employees and other fields. Also, I am having a property which returns a bool, indicating whether an employee exists or not. I am a bit confused regarding the naming of this property.

class Employee_Details
{
  #region Private Fields
  private List<Employee> employees;
  #endregion

  #region Fields
  // Gets a value indicating whether an employee exists
  public bool DoesEmployeeExists  // what should be the name of this property?
  {
    get
    {
      return this.employees.Any();
    }
  }
  #endregion      
}

Is the property name DoesEmployeeExists correct, or should I use any other name?

\$\endgroup\$
  • 5
    \$\begingroup\$ DoesEmployeeExists is grammatically incorrect. At least use DoesEmployeeExist. But +1 to Has*/Is*, guitar_freak beat me to it. \$\endgroup\$ – a CVn Sep 7 '13 at 18:47
  • \$\begingroup\$ While at it - properties that do work other than input validation and potentially can take long time to run or throw exception on get should be methods. \$\endgroup\$ – Evgeni Sep 7 '13 at 19:35
22
\$\begingroup\$

I would use simply HasEmployees.

In case of Boolean, I always try to use hasSomething or isSomething. In Java, many libraries use this strategy (it's just a convention). And I like it, because it's so easy to read when you see:

if(annoyingEmployee.isRemovable() && !annoyingEmployee.hasBirthday()) {
  manager.fireEmployee(annoyingEmployee);
} 
\$\endgroup\$
  • 7
    \$\begingroup\$ Sometimes also other prefixes can be used for booleans: e.g. if (employee.canFixBug(bug)) { manager.giveExtraHoursTo(employee); } \$\endgroup\$ – Bakuriu Sep 8 '13 at 7:29
7
\$\begingroup\$
class Employee_Details

This class is badly named. First, .Net naming conventions say you shouldn't use underscore for this, the class should be called EmployeeDetails. Second, the name seems to imply it contains details about a single employee. Better names would be EmployeesDetails or something like EmployeeList.

#region Private Fields

I don't see any reason to use #region here, especially since it's for a single field.

#region Fields

If you're going to have regions, at least don't lie in their descriptions. This #region contains properties, not fields.

// Gets a value indicating whether an employee exists

Consider using XML documentation comments. That way, the IDE (and other tools) can understand them.

public bool DoesEmployeeExists

I think a better name is indicated by the implementation: AnyEmployees. But probably even better name would be a negated one: something like NoEmployees or Empty.

Also consider whether this property actually makes sense. I think a better option would be exposing the private list as something like IEnumerable<Employee> or IReadOnlyList<Employee>, which you probably have to do anyway. That way, any user of your class can run any query they want on the list, and you won't have to create separate properties or methods for each kind of query.

\$\endgroup\$
  • 3
    \$\begingroup\$ I'm not convinced about AnyEmployees, NoEmployees or Empty. I don't see any reason in using a negative name: if you want to check if any employees exist you'd have to use if(!NoEmployees) which isn't very fluent. Empty is ambigious because it might just as well refer to any other property in the class. HasEmployees follows conventions (Has-a, Is-a etc) and makes its purpose clear. Agree on everything else. \$\endgroup\$ – Jeroen Vannevel Sep 7 '13 at 17:12
  • \$\begingroup\$ @ svick, this is not my actual code. I was just confused with the property name that's why I have given random name to other entities. For all the regions and all, I was just trying to make the code readable :). I do need to have properties like this in which I check for empty collection, for WPF bindings. \$\endgroup\$ – MehaJain Sep 7 '13 at 19:17
  • 3
    \$\begingroup\$ @MehaJain Well, the rules of this site say that you should post your actual code. That way, you might get comments about thing that you didn't expect to be problematic too. \$\endgroup\$ – svick Sep 7 '13 at 22:17
2
\$\begingroup\$

First, @svick is absolutely right about the class name, regions and xml comments.

But to address your basic question, I would have the class implement either ICollection or IList depending upon how you use the rest of the class. If neither works for you, consider Count... but I wuld really go with IList, as that would allow them to get the answer without you doing much at all other than providing some wrappers around your private variable.

\$\endgroup\$
  • 2
    \$\begingroup\$ Most of the time, have a property of the appropriate collection type makes more sense over implementing a collection interface. Basically, composition over inheritance. \$\endgroup\$ – svick Sep 7 '13 at 22:19
  • \$\begingroup\$ You're right, I was assuming that the OP had a good reason to make the list private instead of public, and if he does, he would probably want the IReadOnlyList instead. \$\endgroup\$ – jmoreno Sep 8 '13 at 2:25

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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