Gathering IP address and workstation information; does it belong in a state class?

Question

I'm writing an enterprisey utility that collects exception information and writes to the Windows Event Log, sends an email, etc. This utility class will be used by all applications in the corporation: web, BizTalk, Windows Services, etc.

Currently this class:

• holds state given to it via public properties
• calls out to .NET Framework methods to gather information about runtime details. Included are call to various properties and methods from System.Environment, Reflection details, etc.

This implementation has the benefit of allowing all those callers not to have to make these same calls themselves. This means less code for the caller to forget, screw up, etc.

Should this state class (please what's the phrase I'm looking for [like DTO]?) know how to resolve/determine runtime details (like the IP address and machine name that it's running on)?

It seems to me on second thought that it's meant to be a class that should hold state, and not know how to call out to the .NET Framework to find information.

var myEx = new AppProblem{MachineName="Riker"};

//Will get "Riker 10.0.0.1" from property MachineLongDesc
Console.WriteLine("full machine details: " + myEx.MachineLongDesc);

public class AppProblem
{
    public string MachineName{get;set;}
    public string MachineLongDesc{
        get{
            if(string.IsNullOrEmpty(this.MachineName)
            {
                this.MachineName = Environment.MachineName;
            }
            return this.MachineName + " " + GetCurrentIP();
        }
    }

    private string GetCurrentIP()
    {
        return System.Net.Dns.GetHostEntry(this.MachineName)
                     .AddressList.First().ToString();
    }
}

This code was written by hand from memory, and presented for simplicity, trying to illustrate the concept.

Answer 1

In my opinion gathering(i.e. getIp) and processing (i.e. setEmail) should be handled in different classes

• the ip gethering belongs to a log-service that has knowledge about runtime, environment, context, session....
• the processing of the log-infos belongs to a different logging-persistence class that has a similar interface as a write a only repository and either uses a logging dto or or the save method has several parameters

Example

public class LoggingService {
    private ILoggingRepository repository;

    private string getIp() {...}

    public LoggingService(ILoggingRepository repository) {this.repository=repository;}

    public logError(Date date, String message, Exception ex) {
        this.repository.save(date, message, ex, getIp(), getLoggedInUser(), ....);
    }
}

repository.save may be implemented as sendEmail or save to database or createErrorTicket .....

Answer 2

You're creating a level of abstraction between code that generates exceptions, and code that reports exceptions. It seems to me that the interface to the lower level abstraction - that of reporting - should just include accepting objects that represent exceptions with defined methods of getting a human-readable representation.

In general, exceptions should include any context for the problem that isn't available to whatever is handling it, nothing more. If the handler has more context, it may wrap the exception with the additional info and pass it on, or just log it.

In your situation then, the error reporting utility can retrieve the additional context that it's aware of - the runtime details of the code that generated the exception - and include that in its output.

If you're concerned about putting logic in what you think should be kept to a simple DTO, add the logic as an intermediate step between accepting the exception object - maybe this is the DTO - and reporting.

Answer 3

It seems to me on 2nd through that it's meant to be a class that should hold state, and not know how to call out to the .NET Framework to find info.

Spot on. What you have is just a data container. It doesn't need any methods; there's no implementation details to hide. There's a ton of things you could choose to do with that data, and multiple ways of doing each of those things; if you use methods instead of static functions the class will bloat as you add every new little use case you find.

For bonus points, make it immutable and give it value semantics.

Answer 4

I would create a model/business entity/data transfer object/POCO (whatever you wish to call them) class which just holds the data which you have retrieved. I would then create a class which would call the appropriate API's to collect the data and pass back your populated model object to be consumed by the report/saved to persistence medium.

I'm working on quite a large project right now which collects Windows server configuration data. I have built various repository classes to collect the data from the servers using WMI/registry etc which populates a model object according to the area of interest. For example, I have a repository which collects services data populating and returning a ServicesData model, another collects network card data, populating a NicData model ...