patterncsharpMinor

Unit test for a function that adds users

Submitted by: @import:stackexchange-codereview·Mar 10, 2026·

Viewed 0 times

functionthatfortestaddsusersunit

Problem

I'm new to unit testing and I'm curious if anyone can see any problems with my approach below. Basically I want to test the Add function. To do this I am creating a new Account object, passing that to the Add function, retrieving that account and finally comparing the retrieved objects values to hard coded values.

I'm wondering if anyone can see any thing that would not be considered best practice for unit tests? Specifically, should I be using the testUser object in my asserts rather than the hard coded values? Anything else I should consider?

[TestMethod]
public void TestAddUser()
{
    AccountAccess accountActions = new AccountAccess();
    Account testUser = new Account("abe",1000m);
    accountActions.Add(testUser);
    Account retrievedUser = accountActions.GetAccount("abe");
    Assert.AreEqual("abe", retrievedUser.name, "Added user does not have corret name: 'abe' - '" + retrievedUser.name + "'");
    Assert.AreEqual(1000m, retrievedUser.limit, "Added user does not have corret limit: '1000' - '" + retrievedUser.limit + "'");
}

Solution

A unit test is split in 3 steps. Arrange-Act-Assert. Usually, to make it clear what is being tested, you write as a comment in which step you're at.

[TestMethod]
public void TestAddUser()
{
    //Arrange
    AccountAccess accountActions = new AccountAccess();
    Account testUser = new Account("abe",1000m);

    //Act
    accountActions.Add(testUser);

    //Assert
    Account retrievedUser = accountActions.GetAccount("abe");
    Assert.AreEqual("abe", retrievedUser.name, "Added user does not have corret name: 'abe' - '" + retrievedUser.name + "'");
    Assert.AreEqual(1000m, retrievedUser.limit, "Added user does not have corret limit: '1000' - '" + retrievedUser.limit + "'");
}

Now, to make your test as good as possible, it needs to be as small as possible but still crystal clear about what you're testing.

What are your Assert testing? If the name and limits are equals.
What do you want your

Assert to test? That the same account that was added is returned. That both accounts are equal.

It's not a big difference, but what if one day the equality of an

Account is based on an Id? You might need to refactor this test because the assertions would be false. Ex : If I add twice the same Account with the same name and limit

, which is supposed to be the one that is returned?

What I mean is, your test should look like this : 

[TestMethod]
public void AddUser()
{
    //Arrange
    AccountAccess accountActions = new AccountAccess();
    Account expected = new Account("abe",1000m);

    //Act
    accountActions.Add(new Account("abe",1000m));

    //Assert
    Account actual = accountActions.GetAccount("abe");
    Assert.AreEqual(expected, actual);
}


Now, this test might fail, if so, it's because your

Account class doesn't override the Equal and GetHashCode

 methods. It probably should as they both  define the identity of your object.

Let's get back to my last example. What is different from your test?

Expected/Actual : This way, I know what are the expected results (I have a variable for that) and what was actually returned (I also have a variable for that!)

Only one assertion. It's a best practice, you can't always achieve one assertion, but that should be your goal. This way, it's easy for anyone to understand what your test... tests. It's now very clear I'm testing that it's the same account that is returned, because internally Assert.AreEqual will use the Equal property of your class. And it seems reasonable that the Account class should know if it's equal to another much better than your unit test.




Now, if you don't want to override the equals, there's another solution for you. Use of constants

Ex : 

[TestMethod]
public void AddUser()
{
    //Arrange
    const string accountName = "abe";
    const decimal accountLimit = 1000m;
    AccountAccess accountActions = new AccountAccess();
    Account testUser = new Account(accountName,accountLimit);

    //Act
    accountActions.Add(testUser);

    //Assert
    Account retrievedUser = accountActions.GetAccount(accountName);
    Assert.AreEqual(accountName, retrievedUser.name);
    Assert.AreEqual(accountLimit, retrievedUser.limit);
}


This way, there's no confusion about "abe" being everywhere, heck, I don't even need to know what is inside the

accountName

 variable, I just need to know it is equals to the retrieved one because I used it to retrieve the account. Actually, the use of constants should be heavily considered in my first example too!!

Note : The string messages, aren't necessary. The default assertion message for

Assert.AreEqual will be very clear that the actual value isn't equal to the expected one. And your test name shouldn't include Test`. You know it's a test, there's the attribute juste above :) (You should take a look at @Konrad's comment about naming!)

Code Snippets

[TestMethod]
public void TestAddUser()
{
    //Arrange
    AccountAccess accountActions = new AccountAccess();
    Account testUser = new Account("abe",1000m);

    //Act
    accountActions.Add(testUser);

    //Assert
    Account retrievedUser = accountActions.GetAccount("abe");
    Assert.AreEqual("abe", retrievedUser.name, "Added user does not have corret name: 'abe' - '" + retrievedUser.name + "'");
    Assert.AreEqual(1000m, retrievedUser.limit, "Added user does not have corret limit: '1000' - '" + retrievedUser.limit + "'");
}

[TestMethod]
public void AddUser()
{
    //Arrange
    AccountAccess accountActions = new AccountAccess();
    Account expected = new Account("abe",1000m);

    //Act
    accountActions.Add(new Account("abe",1000m));

    //Assert
    Account actual = accountActions.GetAccount("abe");
    Assert.AreEqual(expected, actual);
}

[TestMethod]
public void AddUser()
{
    //Arrange
    const string accountName = "abe";
    const decimal accountLimit = 1000m;
    AccountAccess accountActions = new AccountAccess();
    Account testUser = new Account(accountName,accountLimit);

    //Act
    accountActions.Add(testUser);

    //Assert
    Account retrievedUser = accountActions.GetAccount(accountName);
    Assert.AreEqual(accountName, retrievedUser.name);
    Assert.AreEqual(accountLimit, retrievedUser.limit);
}

Context

StackExchange Code Review Q#108735, answer score: 7

Revisions (0)

No revisions yet.