Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Is this good C# style?

Consider the following method signature:

public static bool TryGetPolls(out List<Poll> polls, out string errorMessage)

This method performs the following:

  • accesses the database to generate a list of Poll objects.
  • returns true if it was success and errorMessage will be an empty string
  • returns false if it was not successful and errorMessage will contain an exception message.

Is this good style?

Update: Lets say i do use the following method signature:

public static List<Poll> GetPolls()

and in that method, it doesn't catch any exceptions (so i depend the caller to catch exceptions). How do i dispose and close all the objects that is in the scope of that method? As soon as an exception is thrown, the code that closes and disposes objects in the method is no longer reachable.

like image 877
burnt1ce Avatar asked Aug 13 '09 20:08

burnt1ce


2 Answers

That method is trying to do three different things:

  1. Retrieve and return a list of polls
  2. Return a boolean value indicating success
  3. Return an error message

That's pretty messy from a design standpoint.

A better approach would be to declare simply:

public static List<Poll> GetPolls()

Then let this method throw an Exception if anything goes wrong.

like image 87
VoteyDisciple Avatar answered Oct 24 '22 13:10

VoteyDisciple


This is definitely not an idiomatic way of writing C#, which would also mean that it probably isn't a good style either.

When you have a TryGetPolls method then it means you want the results if the operation succeeds, and if it doesn't then you don't care why it doesn't succeed.

When you have simply a GetPolls method then it means you always want the results, and if it doesn't succeed then you want to know why in the form of an Exception.

Mixing the two is somewhere in between, which will be unusual for most people. So I would say either don't return the error message, or throw an Exception on failure, but don't use this odd hybrid approach.

So your method signatures should probably be either:

IList<Poll> GetPolls();

or

bool TryGetPolls(out IList<Poll> polls);

(Note that I'm returning an IList<Poll> rather than a List<Poll> in either case too, as it's also good practice to program to an abstraction rather than an implementation.)

like image 24
Greg Beech Avatar answered Oct 24 '22 14:10

Greg Beech