Should I Use a Method with Parameters or a Function?

Question

I've got a philosophical programming problem. Let's say I have a class named Employees. Employees has business members that get set from a dataTable. In order to fill this, I use a method that takes an instance of the employee class, loops through a dataTable, and sets the members of the instance passed into it. For instance:

public void GetEmployees(objEmployee)
{
  //the function I am calling returns a dataTable of all the employees in the db.
  dim dt as DataTable = dbEmployees.GetEmployees(); 

   foreach(DataRow drow in dt.rows) 
     {
        objEmployee.Name = drow["Name"].ToString();
        objEmployee.ID = drow["ID"].ToString();
     }
}

Then I would call the code like this in my UI logic:

public void GetEmployees()
{
   Employees employee = new Employees();

   employee.GetEmployees(employee);
}

My question is, is it acceptable to pass in my class instance into a method and change the properties like I am doing, or would it be more object-oriented to do it through a function like this:

 public Employees GetEmployees()
  {
     Employees objEmployee = new Employees();

  //the function I am calling returns a dataTable of all the employees in the db.
  dim dt as DataTable = dbEmployees.GetEmployees(); 

   foreach(DataRow drow in dt.rows) 
     {
        objEmployee.Name = drow["Name"].ToString();
        objEmployee.ID = drow["ID"].ToString();
     }

  return objEmployee


}

And then I would call it like this:

private void GetEmployees()
{

Employees employee;

employee = employee.GetEmployees();
}

Is there any advantage of using a function over a method? Thanks!

Answer 1

Sounds to me like you ought to make GetEmployees() a static method off of Employee. You shouldn't have to instantiate an employee to get a list of them.

Also, your first example is only going to set your objEmployee to whatever comes up last in your data. While it loops through all the employees, it stops looping when it reaches the last one, which is the data you'll get returned.

Also, does the "Employees" class refer to one employee or to many? Name it accordingly. If "Employees" represents one "Employee" then perhaps you should rename it to "Employee" and return a List from the GetEmployees method, which, as I stated above, ought to be static, so you can simply call something like "Employee.GetEmployees()".

That being said, I'm never too fond of the architecture where you provide data access capabilities to your business object. It tends to couple the data access layer and the business object layer too tightly.

You may want to consider creating a Data Access interface that accepts parameters for searching for employees and returns actual Employee objects as it's result. Once you do that, you would want to create an implementation of this Data Access Layer that you would then use to generate the actual instances. The advantage to this would be that you could quickly change your implementation of the Data Access Layer without having to change the business objects as well. You would program your business objects off of the Interface then, and you might be able to use dynamic assembly loading or some other method to dynamically determine the implementation of your data access.

Answer 2

Both things are methods (also known as functions). The difference is that the first one "returns by reference" while the second one "returns a reference".

There is no advantage in returning by reference in C# because in the simpler, natural case where you merely return a reference, no copying is done (unlike in C++).

Returning a reference is, thus, to be always preferred as it's the easiest, and it allows great syntactic flexibility at the call site (such as nesting expressions: manager.Fire(GetEmployee()) without the need for a separate statement).

Answer 3

public void GetEmployees(objEmployee)
{
  //the function I am calling returns a dataTable of all the employees in the db.
  dim dt as DataTable = dbEmployees.GetEmployees(); 

   foreach(DataRow drow in dt.rows) 
     {
        objEmployee.Name = drow["Name"].ToString();
        objEmployee.ID = drow["ID"].ToString();
     }
}

In your first approach what would you do if there is no Employees? You created Employe object and pass the method to fill it and then you want to check if there are employees. But when you want to check there will be never null value because you sent created object.I think second one is better and more understanable.

Employees employee = new Employees();

 employee.GetEmployees(employee);

 if(employee==null)//but employee is not null??
     DoSomething();