Function to validate a GUID, an EntityState, and some data

Question

In the beginning of my methods that handle the calls from outside, I run this method to make sure that everything's OK, or let the caller know what's wrong:

private static void Validate(Guid userId, EntityState state, object data)
{
  if (userId == Guid.Empty)
    throw new Exception("Token unrecognized!");
  if (state == EntityState.Detached)
    throw new Exception("Action unrecognized!");
  if (data == null)
    throw new Exception("Data unrecognized!");
}

I was happy with the method until R# remarked that I'm declaring the variables without using them, except for pre-condition check. Well, it's true that I do. It's kind of the whole idea.

It bothers me that the tool remarks on it, because it feels like smarter people than me have put in the message there. However, turning it off isn't an option because that'd turn off even the checks for actually unnecessarily declared variables.

Should I refactor the code? How can I improve it?

Answer 1

It makes sense to use these arguments and throw ArgumentException or ArgumentNullException instead of just Exception:

private static void Validate(Guid userId, EntityState state, object data)
{
    if (userId == Guid.Empty)
        throw new ArgumentException("Token unrecognized!", nameof(userId));
    if (state == EntityState.Detached)
        throw new ArgumentException("Action unrecognized!", nameof(state));
    if (data == null)
        throw new ArgumentNullException(nameof(data), "Data unrecognized!");
}

Since the nameof() is a C#6 feature, in the previous versions of C# you should use argument name string instead: "userId", "state", "data".

Answer 2

Besides Dmitry's suggestion and based on EBrown's comment, it makes sense to log more detailed information. This can be provided using a formatted exception string or using a logging library.

The second option has the advantage that can be customized to log only some levels (e.g. one usually wants less verbosity on production). The code using NLog (but this can be replaced with any other library or a custom one) would look like this:

private static void Validate(Guid userId, EntityState state, object data)
{
    String errorFormat = "{0} for user {1}, entity state {2}, object data = {3}";

    if (userId == Guid.Empty)
    {
        logger.Error(errorFormat, "Token unrecognized!", userId, state, data);
        throw new ArgumentException("Token unrecognized!", nameof(userId));
    }    

    if (state == EntityState.Detached)
    {
        logger.Error(errorFormat, "Action unrecognized!", userId, state, data);
        throw new ArgumentException("Action unrecognized!", nameof(state));
    }

    if (data == null)
    {
        logger.Error(errorFormat, "Data unrecognized!", userId, state, data);
        throw new ArgumentNullException(nameof(data), "Data unrecognized!");
    }
}