c# search and return values

  • Thread starter Thread starter Eric the half a Bee
  • Start date Start date
E

Eric the half a Bee

Hello

I am trying to implement a search function within a collection of
Employees. I am searching for a specific EmpID number in the collection,
and if it is found, I want to return the Employee. This I can do.

The problem comes if the EmpID is not found in the collection. The only way
to achieve this I can think of is to throw an invalid argument exception:


public Employee WithID(int EmpID)

{

try

{

foreach(Employee employee in personnel)

{

if (employee.EmpID == employee.EmpID)

return employee;

}

throw new ArgumentException();

}

catch (ArgumentException ae)

{

Console.Out.WriteLine("Invalid argument passed. EmpID not in
records.{0}",ae.ToString());

Console.Out.WriteLine(ae);

}

finally

{

return new Manager();

}


}



This of course doesn't work either, as control apparently cannot leave a
finally clause.



Any suggestions?



Thanks
 
Hmm, how about linking Employee objects with their IDs in a Hashtable?
So before getting an employee, you can just check whether the key is
already there, and you can throw an exception.

I don't get what you're saying when you say control cannot leave
finally. What do you mean?

Cheers
Elder
 
Why not returning null to signal non-existance.
It doesn't sound right (to me) in this situation to throw an exception.
Also, you should not handle the exception within your search routine,
but let the exception pass to the caller of the search routine.
Like this the exception does not make much sense.

I would do something like this:

public Employee WithID(int EmpID)
{
foreach (Employee employee in personnel)
{
if (employee.EmpID == EmpID)
return employee.
}

// use one of these three !
return null; // to signal non-existance through the return value.
throw new ArgumentException(.....); // to signal non-existance by
throwing an exception.
return new Manager(EmpID); // return a new employee for EmpID.
}

The last option (returning a new employee) is not a good solution. You
don't know if your search
function returned an existing employee or a new employee. While (probably)
you still have to insert
it in personnel.
I should just return null.

Greetings,
Bram.
 
Hello Eric,

Your routine is unusual, I must say.

First off, it does not make sense to raise an error at the end of a block of
code that you promptly catch. You are not catching any other errors, just
the one that you, yourself, raised.. That's not what it's for, and the code
is more complicated, difficult to follow.

Secondly, you have no reason to return from within a Finally block.

Drop the try catch block altogether.

Secondly, your design is not clear. What is the method responsible for? If
it is responsible for returning the object from a list where the object is
expect to exist, raise an error and let the calling code catch it. If it
responsible for searching a list and, if found, returning a value, then
return the value or a sentinel value (like NULL) that can be detected in the
calling code (sentinels have fallen out of favor in recent years because too
many times, errors occur when the calling code doesn't check the return).
Of the two, I prefer the idea of raising an error, but that means that you
let the calling code catch it.

Third, Console.WriteLine? Are you writing a console app? If so, this makes
sense. If not, consider using a logging object or a debug message handler.
The problem with Console.WriteLine is that these things tend to linger long
after you need them.

Fourth, what kind of Collection is it? Many collections have the ability to
find members by keys... why not use one of them, rather than having to write
your own search routine at all.

Fifth, why create a new manager? Seems like the responsibility of your
method is to return an employee object, no matter what. However, that seems
like an unlikely responsiblity. You are more likely to introduce a bug this
way.

So, my suggestions lie with the other responders:

public Employee WithID(int EmpID)
{
foreach(Employee employee in personnel)
{
if (employee.EmpID == employee.EmpID)
return employee;
}
throw new ApplicationException("Employee " + EmpID.ToString() + " not
found");
}

Calling code:
Employee emp;
try
{
emp = this.WithID(currentid);
// do something with the employee found
}
catch
{
// decide now if you want to create a new manager object.
emp = new Employee();
}

Hope this helps,
--- Nick Malik
 
Back
Top