CODEWITHSUNDEEP
c#parametersnull
jrpz
jrpz

Reputation: 93

Should methods handle nulls? best practice in this case?

I have the following situation in code, whats the best way to manage it, the comments contains the situations, and please recommend the best practice.

try
{
    string errorMessage = AccountClient.GetAccount(id, out accountDetails);

    // FIRST WAY : REMOVE THIS NULL CHECK AT ALL AND LEAVE GetAccountDetails to control
    // the Null situation?
    if (accountDetails == null)
    {
        // Second Way: This way? Throw exception here?
        throw new ArgumentNullException(nameof(accountDetails));
        //Third way? break the function?
        break;
    }

    // GetAccount Details already has null control
    Subscription subscription = AccountProcessor.GetAccountDetails(accountDetails);
}
catch (Exception e)
{
    throw;
}

Upvotes: 3

Views: 109

Answers (3)

Marlon
Marlon

Reputation: 1877

It depends on where this ID is coming from. If the user typed the ID, then I wouldn't generate an Exception, since it is not a error in your program. Just treat the user input and show a proper message. Exceptions are costly, so I usually use them only when i have a real programa failure. Besides that, if you write a custom Exception Handler, it wouldn`t make sense to log a error caused by wrong user input. So i would make it like this: 

if (AccountClient.AccountExists(id))
{
    AccountDetails details = AccountClient.GetAccount(id);
    Subscription subscription = AccountProcessor.GetAccountDetails(accountDetails);
}

Anyway, its good to treat the input on the same way, even if you had treated like above, in case there is any other non treated call to it:

public AccountDetails GetAccount(int id)
{
    if (Exists(id))
        GetTheAccount(id);
    else
        throw new Exception(String.Format("Account {0} doesn't exists", id));
}

In this case I would use an Exception because it could really represent an error, if the caller function is passing a wrong value, for instance.

Upvotes: 1

Dmitrii Bychenko
Dmitrii Bychenko

Reputation: 186813

First of all, the costruction 

catch (Exception e) {
  throw; 
}

is redundant one and can be eliminated. Now about nulls. There're two cases:

  • null is an erroneous value and so it should be signalled
  • null is an expected, ordinary value and thus it should be proceeded

And so you have (null is an error)

string errorMessage = AccountClient.GetAccount(id, out accountDetails);

// What's wrong: it's id which doesn't correspond to any detail 
// (we expect id being s.t. AccountClient.GetAccount(id...) returns not null detail)
if (accountDetails == null) 
  throw new ArgumentException($"Incorrect id {id} which doesn't have any detail.", 
                              nameof(id));

Subscription subscription = AccountProcessor.GetAccountDetails(accountDetails);

Or (null is an expected outcome)

string errorMessage = AccountClient.GetAccount(id, out accountDetails);

if (accountDetails == null)
  return null; // or any reasonable value, or just return, or create new Subscription

Subscription subscription = AccountProcessor.GetAccountDetails(accountDetails);

Upvotes: 3

Hamid Pourjam
Hamid Pourjam

Reputation: 20764

If you can do anything about null input then handle it.

try
{
    string errorMessage = AccountClient.GetAccount(id, out accountDetails);

    if (accountDetails == null)
    {
         // do something about it. Maybe write some logs, substitute with a default value
         // or throw appropriate exception ...
    }

    Subscription subscription = AccountProcessor.GetAccountDetails(accountDetails);
}
catch (Exception e)
{
    throw;
}

if you can't then let GetAccountDetails decide what should happen.

try
{
    string errorMessage = AccountClient.GetAccount(id, out accountDetails);
    Subscription subscription = AccountProcessor.GetAccountDetails(accountDetails);
}
catch (Exception e)
{
    throw;
}

Also there is no need to catch an exception, doing nothing and then throw it so you can remove the whole try catch block.

Upvotes: 1

Related Questions