49

Suppose I have a custom object, Student:

public class Student{
    public int _id;
    public String name;
    public int age;
    public float score;
}

And a class, Window, that is used to show information of a Student:

public class Window{
    public void showInfo(Student student);
}

It looks quite normal, but I found Window is not quite easy to test individually, because it needs a real Student object to call the function. So I try to modify showInfo so that it does not accept a Student object directly:

public void showInfo(int _id, String name, int age, float score);

so that it is easier to test Window individually:

showInfo(123, "abc", 45, 6.7);

But I found the modified version has another problems:

  1. Modify Student (e.g.:add new properties) requires modifying the method-signature of showInfo

  2. If Student had many properties, the method-signature of Student would be very long.

So, using custom objects as parameter or accept each property in objects as parameter, which one is more maintainable?

  • 40
    And your 'improved' showInfo requires a real String, a real float and two real ints. How is providing a real String object better than providing a real Student object? – Bart van Ingen Schenau May 12 '16 at 7:08
  • 28
    One major problem with passing the parameters in directly: you now have two int parameters. From the call site, there's no verification that you're actually passing them in the right order. What if you swap id and age, or firstName and lastName? You're introducing a potential point of failure that can be very hard to detect until it blows up in your face, and you're adding it at every call site. – Chris Hayes May 12 '16 at 7:30
  • 38
    @ChrisHayes ah, the old showForm(bool, bool, bool, bool, int) method - I love those... – Boris the Spider May 12 '16 at 8:02
  • 3
    @ChrisHayes at least its not JS ... – Jens Schauder May 12 '16 at 8:23
  • 2
    an underappreciated property of tests: if it's hard to create/use your own objects in tests, your API could possibly use some work :) – Eevee May 13 '16 at 3:53

10 Answers 10

131

Using a custom object to group related parameters is actually a recommended pattern. As a refactoring, it is called Introduce Parameter Object.

Your problem lies elsewhere. First, generic Window should know nothing about Student. Instead, you should have some kind of StudentWindow that knows about only displaying Students. Second, there is absolutely no problem about creating Student instance to test StudentWindow as long as Student doesn't contain any complex logic that would drastically complicate testing of StudentWindow. If it does have that logic then making Student an interface and mocking it should be preferred.

  • 14
    Worth warning that you can get yourself into trouble if the new object isn't really a logical grouping. Don't try to shoehorn every parameter set into a single object; decide on a case by case basis. The example in the question seems pretty clearly to be a good candidate for it. The Student grouping makes sense and is likely to crop up in other areas of the app. – jpmc26 May 12 '16 at 8:56
  • Pedantically speaking if you already have the object, e.g. Student, it would be a Preserve Whole Object – abuzittin gillifirca May 12 '16 at 10:38
  • 4
    Also remember the Law of Demeter. There's a balance to be struck but the tldr is don't do a.b.c if your method takes a. If your method gets to the point where you need to have roughly more than 4 parameters or 2 levels deep of property accession, it probably needs to be factored out. Also note that this is a guideline - like all other guidelines, it requires user discretion. Don't follow it blindly. – Dan Pantry May 12 '16 at 15:45
  • 7
    I found the first sentence of this answer exceedingly difficult to parse. – helrich May 12 '16 at 17:47
  • 5
    @Qwerky I would very strongly disagree. Student REALLY sounds like a leaf in the object graph (barring other trivial objects like maybe Name, DateOfBirth etc), merely a container for state of a student. There is no reason whatsoever that a Student should be hard to construct, because it should be a record type. Creating test doubles for a student sounds like a recipe for hard to maintain tests and/or heavy dependence on some fancy isolation framework. – sara May 13 '16 at 10:18
26

You say it's

not quite easy to test individually, because it needs a real Student object to call the function

But you can just create a student object to pass to your window:

showInfo(new Student(123,"abc",45,6.7));

It doesn't seem much more complex to call.

  • 7
    The problem comes when Student refers to a University, which refers to many Facultys and Campuss, with Professors and Buildings, none of which showInfo actually uses, but you've not defined any interface that allows the tests to "know" that and supply only the relevant student data, without building the whole organisation. The example Student is a plain data object and, like you say, tests should be happy to work with it. – Steve Jessop May 12 '16 at 9:44
  • 4
    The problem comes when Student refers to a University, which refers to many Facultys and Campuss, with Professors and Buildings, No rest for the wicked. – abuzittin gillifirca May 12 '16 at 9:53
  • 1
    @abuzittingillifirca, "Object Mother" is one solution, also your student object may be too complex. It may be better just to have UniversityId, and a service (using dependency injection) that will give a University object from a UniversityId. – Ian May 12 '16 at 10:43
  • 12
    If Student is very complex or is hard to initialize just mock it. Testing is much more powerfull with frameworks like Mockito or other languages equivalents. – Borjab May 12 '16 at 11:29
  • 4
    If showInfo doesn't care about University, then simple set it to null. Nulls are horrible in production and a god-send in tests. Being able to specify a parameter as null in a tests communicates intent and says that "this thing isn't needed here". Although I'd also consider creating some sort of view modelfor students containing only the relevant data, considering that showInfo sounds like a method in a UI class. – sara May 13 '16 at 10:20
22

In layman's terms:

  • What you call a "custom object" is usually simply called an object.
  • You cannot avoid passing objects as parameters when designing any non-trivial program or API, or using any non-trivial API or library.
  • It's perfectly OK to pass objects as parameters. Take a look at the Java API and you will see lots of interfaces that receive objects as parameters.
  • The classes in the libraries you use where written by mere mortals like you and me, so the ones we write are not "custom", they just are.

Edit:

As @Tom.Bowen89 states it isn't much more complex to test the showInfo method:

showInfo(new Student(8812372,"Peter Parker",16,8.9));
3
  1. In your student example I assume it is trivial to call the Student constructor to create a student to pass in to showInfo. So there is no problem.
  2. Assuming the example Student is deliberately trivialised for this question and it is more difficult to construct, then you could use a test double. The are a number of options for test doubles, mocks, stubs etc. that are talked about in Martin Fowler's article to choose from.
  3. If you want to make the showInfo function more generic you could have it iterate over the public variables, or maybe the public accessors of the object passed in and perform the show logic for all of them. Then you could pass in any object that conformed to that contract and it would work as expected. This would be a good place to use an interface. For example pass in a Showable or ShowInfoable object to the showInfo function that can show not just students info but any object's info that implements the interface (obviously those interfaces need better names depending on how specific or generic you want the object you can pass in to be and what a Student is a sub class of).
  4. It is often easier to pass around primitives, and sometimes necessary for performance, but the more you can group similar concepts together the more understandable your code will generally be. The only thing to watch out for is to try not to over do it and end up with enterprise fizzbuzz.
3

Steve McConnell in Code Complete addressed this very issue, discussing the benefits and drawbacks of passing objects into methods instead of using properties.

Forgive me if I get some of the details wrong, am working from memory as it's been over a year since I had access to the book:

He comes to the conclusion that you are better off not using an object, instead sending only those properties absolutely necessary for the method. The method should not have to know anything about the object outside of the properties it will be using as part of its operations. Also, over time, if the object is ever changed, this could have unintended consequences on the method using the object.

Also he addressed that if you end up with a method which accepts a lot of different arguments, then that is probably a sign that the method is doing too much and should be broken down into more, smaller methods.

However, sometimes, sometimes, you actually do need a lot of parameters. The example he gives would be of a method that builds a full address, using many different address properties (though this could be gotten around by using a string array when you think about it).

  • 7
    I have the Code Complete 2. There's a whole page in it dedicated to this issue. The conclusion is that parameters should be at the correct abstraction level. Sometimes that requires passing a whole object, sometimes just individual attributes. – COME FROM May 12 '16 at 11:17
  • UV. Referencing Code Complete is double-plus good. A nice consideration of design over testing expediency. Prefer design, I think McConnell would say in our context. So an excellent conclusion would be "integrate the parameter object into the design" (of Student in this case). And this is how testing informs design, completely embracing the most-votes answer while maintaining design integrity. – radarbob May 12 '16 at 14:58
2

It is much easier to write and read tests if you pass the whole object:

public class AStudentView {
    @Test 
    public void displays_failing_grade_warning_when_a_student_with_a_failing_grade_is_shown() {
        StudentView view = aStudentView();
        view.show(aStudent().withAFailingGrade().build());
        Assert.that(view, displaysFailingGradeWarning());
    }

    private Matcher<StudentView> displaysFailingGradeWarning() {
        ...
    }
}

For comparison,

view.show(aStudent().withAFailingGrade().build());

line could be written, if you pass values separately, as:

showAStudentWithAFailingGrade(view);

where actual method call is buried somewhere like

private showAStudentWithAFailingGrade(StudentView view) {
    int someId = .....
    String someName = .....
    int someAge = .....
    // why have been I peeking and poking values I don't care about
    decimal aFailingGrade = .....
    view.show(someId, someName, someAge, aFailingGrade);
}

To be to the point, that you cannot put the actual method call in the test is a sign that you API is bad.

1

You should pass what makes sense, some ideas:

Easier to test. If the object(s) need to be edited, what requires the least refactoring? Is re-using this function for other purposes useful? What is the least amount of information I need to give this function to do it's purpose? (By breaking it up--it may let you re-use this code--be wary of falling down the design hole of making this function and then bottlenecking everything to exclusively use this object.)

All these programming rules are just guides to get you thinking in the right direction. Just don't build a code beast--if you're unsure and just need to proceed, pick a direction/your own or a suggestion here, and if you hit a point where you think 'oh, I should have made it this way'--you probably can then go back and refactor it pretty easily. (For instance if you have Teacher class--it just needs the same property set as Student, and you change your function to accept any object of the Person form)

I would be most inclined to keep the main object being passed--because how I code it's going to more easily explain what this function is doing.

1

One common route around this is to insert an interface between the two processes.

public class Student {

    public int id;
    public String name;
    public int age;
    public float score;
}

interface HasInfo {
    public String getInfo();
}

public class StudentInfo implements HasInfo {
    final Student student;

    public StudentInfo(Student student) {
        this.student = student;
    }

    @Override
    public String getInfo() {
        return student.name;
    }

}

public class Window {

    public void showInfo(HasInfo info) {

    }
}

This gets a little messy sometimes but things get a little tidier in Java if you use an inner class.

interface HasInfo {
    public String getInfo();
}

public class Student {

    public int id;
    public String name;
    public int age;
    public float score;

    public HasInfo getInfo() {
        return new HasInfo () {
            @Override
            public String getInfo() {
                return name;
            }

        };
    }
}

You can then test the Window class by just giving it a fake HasInfo object.

I suspect this is an example of the Decorator Pattern.

Added

There seems to be some confusion caused by the simplicity of the code. Here's another example that may demonstrate the technique better.

interface Drawable {

    public void Draw(Pane pane);
}

/**
 * Student knows nothing about Window or Drawable.
 */
public class Student {

    public int id;
    public String name;
    public int age;
    public float score;
}

/**
 * DrawsStudents knows about both Students and Drawable (but not Window)
 */
public class DrawsStudents implements Drawable {

    private final Student subject;

    public DrawsStudents(Student subject) {
        this.subject = subject;
    }

    @Override
    public void Draw(Pane pane) {
        // Draw a Student on a Pane
    }

}

/**
 * Window only knows about Drawables.
 */
public class Window {

    public void showInfo(Drawable info) {

    }
}
  • If showInfo only wanted to display the name of the student, why not just pass the name? wrapping a semantically meaningful named field in an abstract interface containing a string with no clue as to what the string represents feels like a HUGE downgrade, both in terms of maintainability and understandability. – sara May 13 '16 at 10:26
  • @kai - Use of Student and String here for the return type is purely for demonstration. There would likely be additional parameters to getInfo such as the Pane to draw to if drawing. The concept here is to pass functional components as decorators of the original object. – OldCurmudgeon May 13 '16 at 10:30
  • in that case you'd be coupling the student entity tightly to your UI framework, that sounds even worse ... – sara May 13 '16 at 10:34
  • 1
    @kai - Quite the opposite. My UI only knows about HasInfo objects. Student knows how to be one. – OldCurmudgeon May 13 '16 at 10:36
  • If you give getInfo return void pass it a Pane to draw to, then the implementation (in the Student class) is suddenly coupled to swing or whatever you're using. If you make it return some string and take 0 parameters, then your UI won't know what to do with the string without magic assumptions and implicit coupling. If you make getInfo actually return some view model with relevant properties, than your Student class is again coupled to presentation logic. I don't think any of these alternatives are desirable – sara May 13 '16 at 10:44
1

You have a lot of good answers already, but here are a few more suggestions that may allow you to see an alternative solution:

  • Your example shows a Student (clearly a model object) being passed to a Window (apparently a view-level object). An intermediary Controller or Presenter object may be beneficial if you don't already have one, allowing you to isolate your user interface from your model. The controller/presenter should provide an interface that can be used to replace it for UI testing, and should use interfaces to refer to model objects and view objects to be able to isolate it from both of those for testing. You may need to provide some abstract way of creating or loading these (eg Factory objects, Repository objects, or similar).

  • Transfering relevant parts of your model objects into a Data Transfer Object is a useful approach for interfacing when your model becomes too complex.

  • It may be that your Student violates the Interface Segregation Principle. If so, it could be beneficial to split it into multiple interfaces that are easier to work with.

  • Lazy Loading can make construction of large object graphs easier.

0

This is actually a decent question. The real issue here is the use of the generic term "object", which can be a bit ambiguous.

Generally, in a classical OOP language, the term "object" has come to mean "class instance". Class instances can be pretty heavy - public and private properties (and those in between), methods, inheritance, dependencies, etc. You wouldn't really want to use something like that to simply pass in some properties.

In this case, you're using an object as a container that simply hold some primitives. In C++, objects such as these were known as structs (and they still exist in languages like C#). Structs were, in fact, designed exactly for the usage of which you speak - they grouped related objects and primitives together when they had a logical relationship.

However, in modern languages, there's really no difference between a struct and a class when you're writing the code, so you're okay using an object. (Behind the scenes, however, there are some differences which you should be aware of - for instance, a struct is a value type, not a reference type.) Basically, as long as you keep your object simple, it'll be easy to test manually. Modern languages and tools allow you to mitigate this quite a bit, though (via interfaces, mocking frameworks, dependency injection, etc.)

  • 1
    Passing a reference isn't expensive, even if the object is a billion terabytes large, because the reference is still only the size of an int in most languages. You should worry more about whether the receiving method is exposed to too large an api and if you couple things in an undesirable way. I'd consider making a mapping layer that translates business objects (Student) into view models (StudentInfo or StudentInfoViewModel etc.), but it might not be necessary. – sara May 13 '16 at 10:38
  • Classes and structures are very different. One is passed by value (meaning the method receiving it gets a copy) and the other is passed by reference (the receiver just gets a pointer to the original). Not understanding this difference is dangerous. – RubberDuck May 14 '16 at 14:18
  • @kai I understand that passing a reference isn't expensive. What I'm saying is that creating a function that requires a full class instance may be more difficult to test depending on the dependencies of that class, its methods, etc - as you'd have to mock that class somehow. – lunchmeat317 May 15 '16 at 14:58
  • I'm personally against mocking just about anything at all except boundary classes that access external systems (file/network IO) or ones that are non-deterministic (e.g. random, system time-based, etc). If a class I'm testing has a dependency that isn't relevant to the current feature being tested, I prefer to just pass null where possible. But if you're testing a method that takes a parameter object, if that object has a bunch of dependencies I'd be worried about the overall design. Such objects should be lightweight. – sara May 15 '16 at 21:41

protected by gnat May 13 '16 at 7:12

Thank you for your interest in this question. Because it has attracted low-quality or spam answers that had to be removed, posting an answer now requires 10 reputation on this site (the association bonus does not count).

Would you like to answer one of these unanswered questions instead?

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