Instantiating different kinds of error records for a database operation

Question

I've written code to be called from Powershell like so:

Rename-DatabaseUser -Name 'someuser' -NewName 'somenewuser'

I'm wondering if there's another way I can approach this. I want to create objects of the same type with slightly differing property values.

Here's the code I'm currently using now (RenameDatabaseUserCmdlet.cs):

namespace Company.Powershell.Module.Cmdlets
{
 using System.Management.Automation;
 [Cmdlet(VerbsCommon.Rename, "DatabaseUser")]
 [Alias("rndbu")]
 public class RenameDatabaseUserCmdlet : PSCmdlet
 {
 [Parameter(ParameterSetName = "Object")]
 public DatabaseUser User { get; set; }
 [Parameter(ParameterSetName = "String")]
 public string Name { get; set; }
 [Parameter(ParameterSetName = "Object")]
 [Parameter(ParameterSetName = "String")]
 public string NewName { get; set; }
 protected override void ProcessRecord()
 {
 DatabaseUser user;
 if (ParameterSetName == "String")
 {
 var ps = PowerShell.Create(RunspaceMode.CurrentRunspace);
 ps.AddCommand("Get-DatabaseUser")
 .AddParameter("Name", Name);
 foreach (var result in ps.Invoke())
 {
 user = result?.BaseObject as DatabaseUser;
 if (user == null)
 {
 // This is where I'm not sure I'm doing it right.
 WriteError(DatabaseErrorRecord.UserNotFound(Name));
 return;
 }
 }
 }
 else
 {
 user = User;
 }
 // Still more to do here...
 }
 }
}

This is how I create the objects. In DatabaseErrorRecord I have multiple static methods which return an ErrorRecord object initialised slightly differently. These are some of the methods:

namespace Company.Powershell.Module
{
 using System;
 using System.Management.Automation;
 class DatabaseErrorRecord : ErrorRecord
 {
 // This is added just so it compiles, which is why I think I'm wrong.
 private DatabaseErrorRecord(Exception exception,
 string errorId,
 ErrorCategory errorCategory,
 object targetObject) : base(exception, errorId, errorCategory, targetObject)
 { }
 public static ErrorRecord UserNotFound(string userName)
 {
 return new ErrorRecord
 (
 new NullReferenceException($"The user '{userName}' could not be found in the database."),
 "UserNotFound",
 ErrorCategory.ObjectNotFound,
 null
 );
 }
 public static ErrorRecord DatabaseNotFound(string databaseName)
 {
 return new ErrorRecord
 (
 new NullReferenceException($"The database '{databaseName}' could not be found on the SQL Server instance."),
 "DatabaseNotFound",
 ErrorCategory.ObjectNotFound,
 null
 );
 }
 public static ErrorRecord PermissionConflict(string permissionA, string permissionB)
 {
 return new ErrorRecord
 (
 new InvalidOperationException($"Granting {permissionA} to this user would cause a conflict with {permissionB}."),
 "PermissionConflict",
 ErrorCategory.InvalidOperation,
 null
 );
 }
 // Other methods omitted...
 }
}

What is a good way to create objects of the same type, using slightly different arguments?

In each of these methods, when the ErrorRecord is created. The Exception, Error ID and Error Category will all be different. I would create these objects when I need them in code, but there's some instances where I would need the same object twice.

I'm still thinking this is wrong since I have to include the constructor despite the fact I will never use it.

Is there any other ways I could structure this?

Answer 1

There are no established scenarios for deriving from this class.

MSDN ErrorRecord

Which leads me to believe that there's no reason to derive a child class. Instead, change the name of DatabaseErrorRecord to DatabaseErrorRecordFactory and make it static. Do not derive from ErrorRecord. Instead, just have a static class with a bunch of static methods that return ErrorRecords. This will remove the need to have any constructor at all.

public static class DatabaseErrorRecord
{
 public static ErrorRecord UserNotFound(string userName)
 {
 return new ErrorRecord
 (
 new NullReferenceException($"The user '{userName}' could not be found in the database."),
 "UserNotFound",
 ErrorCategory.ObjectNotFound,
 null
 );
 }
 // Other methods omitted...
}

---

Side note: I really like that you're taking advantage of the new C# 6 features. I'm a big fan of the Elvis operator.

One other thing I'd consider changing is that NullReferenceException though. It doesn't feel appropriate. An argument exception might be okay, but I think I'd create my own custom exception for this one.

• Jake: 'Elvis operator'. I chuckled. :) The only reason I'm using NullReferenceException there is because the method can't continue with a null object. I asked a question about custom exceptions before and was told it was a bit pointless if I didn't extend the functionality of the exception because otherwise I was essentially just 'aliasing' the class.
• RubberDuck: Yeah. I don't have very good argument against NRE other than it could be misleading. I expect NRE's to be a certain class of coding errors. This use of it would surprise me.
• RobH: KeyNotFoundException might be a reasonable choice.
• RubberDuck: Good call @RobH, but I'm thinking the fact that we're having this much trouble finding one in the framework may justify a custom one.
• Jake: @RubberDuck, maybe just create my own ObjectNotFoundException then? Since 'object' could refer to a database, user, domain account, role or permission. I won't be using Entity Framework in this project so hopefully it'll never clash.

Answer 2

Targeting only the ProcessRecord() method.

You have a flaw here, because if the given Name is not found then the resulting Collection<PSObject> from the call to ps.Invoke() won't likely contain any items, hence the foreach won't iterate. This will result in user being null.
The other thing I am concerned is, if it is possible that the user's name isn't unique, the user variable will be overwritten for each iteration of the loop.

If the Name is unique, you shouldn't use a foreach at all and change the code to

 var result = ps.Invoke();
 if (result.count == 0)
 {
 // Database user not found
 // write your error
 return;
 }
 // assuming unique `Name`
 user = (DatabaseUser)result[0].BaseObject;

I have replaced the soft cast with a regular cast because IMO we can be sure that a DatabaseUser is returned at this point.

• Jake: That's actually pretty clever, thanks for that. Name is unique in this case because SQL Server won't let you create identical user names.