10
\$\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?

Jamal
35.2k13 gold badges134 silver badges238 bronze badges
asked Sep 7, 2013 at 14:57
\$\endgroup\$
2
  • 5
    \$\begingroup\$ DoesEmployeeExists is grammatically incorrect. At least use DoesEmployeeExist. But +1 to Has*/Is*, guitar_freak beat me to it. \$\endgroup\$ Commented Sep 7, 2013 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\$ Commented Sep 7, 2013 at 19:35

3 Answers 3

23
\$\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);
} 
answered Sep 7, 2013 at 16:51
\$\endgroup\$
1
  • 7
    \$\begingroup\$ Sometimes also other prefixes can be used for booleans: e.g. if (employee.canFixBug(bug)) { manager.giveExtraHoursTo(employee); } \$\endgroup\$ Commented Sep 8, 2013 at 7:29
8
\$\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.

answered Sep 7, 2013 at 15:32
\$\endgroup\$
3
  • 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\$ Commented Sep 7, 2013 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\$ Commented Sep 7, 2013 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\$ Commented Sep 7, 2013 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.

answered Sep 7, 2013 at 18:38
\$\endgroup\$
2
  • 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\$ Commented Sep 7, 2013 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\$ Commented Sep 8, 2013 at 2:25

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.