Return to Answer

Your class will be difficult to test since you don't implement Inversion of Control. In particular due to this line in the constructor:

_nsoWebService = new NSO { Credentials = GetCredentials() };

But the same applies to Logger _logger = LogManager.GetCurrentClassLogger();: what if you want to write a unit test to check whether the correct message is logged?

There are several instances of this:

if (errorCode != 0)
 HandleErrors(errorCode);
else
 _logger.Trace(/* message */);

I'd rather see this reversed, because testing for a negative makes things more complicated than they should be IMHO (I've also added brackets, something that you should always do):

if (errorCode == 0)
{
 _logger.Trace(/* message */);
}
else
{
 HandleErrors(errorCode);
}

Considering that this log action is the only thing that happens, you could even do this:

if (errorCode == 0)
{
 _logger.Trace(/* message */);
 return;
}
HandleErrors(errorCode);

Your errorCode shouldn't be an int, but an enum with a description.

Isn't there an overload for _logger.Trace() and _logger.Error() that functions the same as string.Format()? Seems there is if you're using NLog.

NSO doesn't follow Microsoft's rules: capitalize only the first character of acronyms with three or more characters. But it is also a class name that doesn't tell me anything.

I also don't think _user and _pass should be in the WebServiceAccess class, nor GetCredentials(). These things belong to NSO.

You do not take into account that an exception may have an inner exception:

_logger.Error("Error updating a technician in Navision. Error: " + ex.Message);

HandleErrors doesn't really do any error handling, now does it? It merely logs the error.

Your class will be difficult to test since you don't implement Inversion of Control. In particular due to this line in the constructor:

_nsoWebService = new NSO { Credentials = GetCredentials() };

But the same applies to Logger _logger = LogManager.GetCurrentClassLogger();: what if you want to write a unit test to check whether the correct message is logged?

There are several instances of this:

if (errorCode != 0)
 HandleErrors(errorCode);
else
 _logger.Trace(/* message */);

I'd rather see this reversed, because testing for a negative makes things more complicated than they should be IMHO (I've also added brackets, something that you should always do):

if (errorCode == 0)
{
 _logger.Trace(/* message */);
}
else
{
 HandleErrors(errorCode);
}

Considering that this log action is the only thing that happens, you could even do this:

if (errorCode == 0)
{
 _logger.Trace(/* message */);
 return;
}
HandleErrors(errorCode);

Your errorCode shouldn't be an int, but an enum with a description.

Isn't there an overload for _logger.Trace() and _logger.Error() that functions the same as string.Format()? Seems there is if you're using NLog.

NSO doesn't follow Microsoft's rules: capitalize only the first character of acronyms with three or more characters. But it is also a class name that doesn't tell me anything.

I also don't think _user and _pass should be in the WebServiceAccess class, nor GetCredentials(). These things belong to NSO.

You do not take into account that an exception may have an inner exception:

_logger.Error("Error updating a technician in Navision. Error: " + ex.Message);

HandleErrors doesn't really do any error handling, now does it? It merely logs the error.