6
\$\begingroup\$

This code is an academic example. I was trying to write something easy because the domain is not important but the principles and best practices.

In this code, I have a class - Table. It has a private array (of objects Cell), which is used by many public functions in the code but there is no function that will return a particular cell. So the problem I have, is how can I test the Reset function? I can call this function but I can't review its result especially with the Option 2 where I don't have any references to this array.

That got me thinking if the design is correct.. and that lead me here.

My questions are:

  1. Do I violate any SOLID principle?
  2. Is the option 2 better way to go? (although in that case, there's no way to test it

Here's the code:

public class Cell
{
    public int X { get; set; }
    public int Y { get; set; }
    public string Value { get; set; }
}

public class Table
{
    private Cell[,] Cells { get; }
    public Table(Cell[,] cells)
    {
        Cells = cells;
    }

    public void SetCell(int x, int y, string value)
    {
        Cells[x, y].Value = value;
    }

    public void Reset()
    {
        for (int i = 0; i < Cells.GetLength(0); i++)
        {
            for (int j = 0; j < Cells.GetLength(1); j++)
            {
                Cells[i, j].Value = "";
            }
        }
    }

    public bool AreNeighborCellsSet(int x, int y)
    {
        bool areNeighborCellsSet = false;

        // checking...

        return areNeighborCellsSet;
    }
}

Option 2:

public class Table
{
    private Cell[,] Cells { get; }

    public Table(int height, int width, ICellFactory cellFactory)
    {
        Cells = new ICell[height, width];
        for (int i = 0; i < Cells.GetLength(0); i++)
        {
            for (int j = 0; j < Cells.GetLength(1); j++)
            {
                Cells[i, j].Value = cellFactory.Create(i, j);
            }
        }
    }

    // Rest is the same
}
\$\endgroup\$
2
  • 2
    \$\begingroup\$ Good job improving the question ;-) \$\endgroup\$
    – t3chb0t
    Apr 14, 2017 at 20:06
  • \$\begingroup\$ It's hard to answer the question as it stands as there is no code listing for ICellFactory. \$\endgroup\$
    – zaitsman
    Apr 17, 2017 at 11:07

3 Answers 3

5
\$\begingroup\$

how can I test Reset function? I can call this function, but I can't review its result. Specially with the Option 2 where I don't have any references to this Array.

You always test only public APIs so you need to verify the results of the many public functions whether they behave they way you expect them after resetting the table.

They are your contract and their expected results you need to verify.


Is the option 2 better way to go? (although in that case, there's no way to test it

There is, the public APIs. If the initialization didn't work correctly you will get invalid results from them.


I don't have any references to this Array

This is an internal detail that could change at any time. You don't test it. You test the public contract that you usually must not change.

\$\endgroup\$
16
  • \$\begingroup\$ hmm.. I've read that a good test has 3 stages - 1. create an object of the class you want to test. 2. Call a function you want to test. 3. Assert the result. Does that mean, that I should call Reset and assert it with a call of another function? \$\endgroup\$ Apr 14, 2017 at 20:51
  • \$\begingroup\$ Wait.. I'm not sure If I understood your 2nd answer about "option 2" \$\endgroup\$ Apr 14, 2017 at 20:51
  • \$\begingroup\$ @LouisaBickley yes, exactly that, you create a Table, initialize it, and test it with one test. Then you create the same table in another test and reset it and verify the results of the APIs again but now expecting perhaps different results. \$\endgroup\$
    – t3chb0t
    Apr 14, 2017 at 20:54
  • 1
    \$\begingroup\$ @LouisaBickley the triple A is just a recommendation, it doesn't mean you cannot test something along the way. If you sometimes need to assert, then change something and assert again then just do it. \$\endgroup\$
    – t3chb0t
    Apr 14, 2017 at 20:57
  • \$\begingroup\$ gotcha! The only think.. can you clarify the "option2" answer? should I used factory in this case or just inject the ready to go array? I have my thoughts on this, but wanted to see your opinion first \$\endgroup\$ Apr 14, 2017 at 21:05
2
\$\begingroup\$

To properly answer your question I would focus firstly on why you want differentiated test sets, and not carry all the eggs in one basket. After that I'll focus on using factories or not, before a little summary at the end. (Sorry in advance, for the somewhat lengthy response.)

Some Test Levels

Different test sets could and should have various focus and coverage area, which again will reveal various kind of errors in your code.

Here is a list of various test areas (with my personal and simplified description):

  • Unit Tests – Test the smallest unit of functionality, like a given method/feature. Should not rely on other components. Often mocking other components is used to avoid dependencies.
  • Functional Tests – Checks specific features in a limited environment, but wider than in the Unit tests. Focus on getting the correct result of a function, with proper error handling, and so on.
  • Integration Tests – Test the combination of units of code. This could be classes, and smaller sequences of code usage. Includes more of the actual environment the software is running within
  • System Tests or Acceptance Tests – Tests within a "production" environment emulating full scale system operations. Depending on system, various tools could be used to emulate the usage of the entire software system.

For a similar description (and most likely better descritption) of various test levels, see https://en.wikipedia.org/wiki/Portal:Software_testing#Topics second line on "Test levels".

Note that the lower test levels here has a higher cost in terms of resource use, and time usage. As such, one tends to automate the upper levels and execute those rather often, whilst the lower levels are executed closer to delivery of the software.

Related to Your Question

The point I'm trying to make is that your question needs to take into account at what level you're testing. For a Unit Test I would test the function of Reset(), but for Integration Test I wouldn't test that particular function but more the overall function of your class.

Whether this violates the SOLID principles or not, is yet again dependent on your focus of testing. To properly unit test the details of a class, maybe some of the SOLID principles will be violated internally to the test framework, but in the integration test they should not be violated.

For example, in multiple larger projects the firm I've worked for has used the Friend Assemblies (C# and Visual Basic from Microsoft. This allows an Unit Test project to access the inner workings of a befriended assembly/project. Aka it allows the Unit Test to access the internal members of the class.

This could be argued that it violates the Open/closed principle and the Liskov substitution principle, but the benefits usually far exceeds the limitations. It does however imply that if you change internal details like from using an Array to using an Simple Linked list, this will require recoding of the Unit Tests (but not the Functional or Integration tests).

Factory or not

One of the advantage of using a Factory approach like you do in Option 2, is that allows for abstraction on a different level. This could be in accordance with the Single responsibility principle and the Dependency inversion principle of SOLID.

It could also shift the test responsibilities over to the factory implementation instead of relying on testing the inner working of your current class. Using a factory could also allow for different integrations at the switch of configuration.

This of course comes at a cost of introducing another layer in your software, and hiding some functionality within the factory. Then again, it also forces you to conform to some standard handling of the data structure (or whatever you're using the factory for), as you only can access it through its public interface).

Related to Your Code

Depending on the size of the Cells[,] structure using a factory to instantiate it would allow for specialization of it depending on size, speed or other important criteria.

Imagining a scenario where this structure holds a large amount of Cells the actual implementation of the structure could change from being held within memory, to accessing databases, using files, using a neural network (if going to the extreme). And done correctly, your code wouldn't know the difference at all. That could be a rather neat feature.

If however, you're always talking about a really small structure, this could be an awful waste of resources.

Regarding your statement: "although in that case, there's no way to test it", well, that is wrong. By introducing a factory implementation, you reduce the Unit Tests on this class, but should introduce another Unit Test of the factory class. It would also introduce a Functionality or Integration Test on the combination.

To Summarize

Firstly, I would strongly advice you to look into Friendly Assemblies, as they provide a great tool for unit testing the internals of your code, without exposing functionality outside of your library.

Secondly, using factories or other means, to extract the internal implementation of the Cells[,], depends on various factors like speed, size, complexity, connectivity, and so on. In some cases, it will really help your implementation.

Thirdly, if you choose to factorize Cells[,], it can still be tested, and in some cases it can even enhance your overall system as you have restrained access to the Cells structure, and thusly limited wrong usage of the structure. And the tests can ensure that the public interfaces does what it should, and nothing else.

\$\endgroup\$
2
  • \$\begingroup\$ Will edit to read "internal", that was a typo... \$\endgroup\$
    – holroy
    Apr 15, 2017 at 19:51
  • \$\begingroup\$ Ok, now I can live with it ;-] \$\endgroup\$
    – t3chb0t
    Apr 15, 2017 at 19:55
1
\$\begingroup\$

This code is an academic example. I was trying to write something easy because the domain is not important but the principles and best practices.

...

That got me thinking if the design is correct.. and that lead me here.

The problem with academic examples is that unless they are thought about to the same level you would think about your production code the usefulness of the example is reduced. To get the most from the site, you're better off posting your actual code (or as close as possible).

Option 2 is broken

As it stands, your option 2 doesn't work. There are many issues with it.

You've defined Cells as Cell[,], however you're initialising it with new ICell[height,width] which doesn't compile. The most likely fix would be to change Cells to ICell[,]. Unless you need to know the concrete type, depending on the abstraction seems to make sense (it's hard to be concrete about that, since you don't include the code for ICell).

You also haven't included the code for ICellFactory. The way that you're calling it however suggests that you're trying to assign the return value from Create into the string value of a cell. This again seems wrong. It should probably be:

Cells[i, j] = cellFactory.Create(i, j);

With Create returning an ICell. It also begs the question, should Create also be taking in a parameter for the value field of the Cell?

Names are important

Good naming reduces the likelihood of confusion and hence bugs. Your naming isn't great. What does AreNeighborCellsSet do? Check for a particular value, any value, same value as the cell tested, it's not clear from the name and again, as you've omitted the code there's no way to know what it's supposed to do.

The bigger problem is that with option 2, you're poor naming has introduced what looks like a bug. You construct your cells using height,width, which is easy enough to follow:

Cells = new Cell[height, width];

Then you initialise each cell:

Cells[i, j].Value = cellFactory.Create(i, j);

This might be ok if Create is expecting (y,x), however I tend to think of positions as (x,y). Either way, using x and y as the variable names for the for loops would have made it easier to have confidence that the code was correct.

Your SetCel method seems to confirm that there is an issue, since it updates the cell:

Cells[x, y].Value = value;

So, you're accessing [height,width] as [x,y]. X is typically the horizontal axis, which I'd associate with width, as opposed to the vertical axis, which I'd associate with height and is usually represented by Y.

Again, how much of this is because it's an academic example is unclear.

There's no way to test option 2?

If you can't test behaviour through the public interface of your class, then it is a good sign that you need to consider refactoring the structure. Is there a reason why you want to allow the clients of the class to explicitly set the value of a given cell, but not retrieve the value? This seems like an artificial constraint, again possibly a side-effect of using academic code.

Without the entire code, it's hard to know for sure, but it seems like you should be able to construct a test like this:

  • Arrange: Use SetCell to setup Cells so that AreNeighborCellsSet would return true if called.

  • Act: Call Reset

  • Assert: That calling AreNeighborCellsSet returns false (I'm assuming this is the case for a reset table).

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.