1

I am designing a test for an existing static method:

public static boolean hasPermissions(String username, int pageid) {
    PermissionsService s = new PermissionsService();
    int[] pages = s.getPages(username);
    for (int i = 0; i < pages.length; i++) {
        if (pages[i] == pageid)
           return true;
    }
    return false;
}

and I want to manipulate the response coming back from the service, and proposed adding another parameter, hasPermissions( .. , PermissionsService ps). However, as there are hundreds of calls to PermissionEngine.hasPermissions across the application, the other teams (who are also using the code) are objecting to this change.

Am I right to think that this is how tests are designs and our static method is tech debt because it wasn't designed with TDD in mind?

Improve this question
1
  • 3
    Am I right to think ... -- Yes, because writing unit tests for this method when it was first written would have identified the hard-wired dependency to PermissionsService. Commented Apr 21, 2021 at 19:46

2 Answers 2

Reset to default
4

Am I right to think that this is how tests are designs and our static method is tech debt because it wasn't designed with TDD in mind?

Arguably.

Personally, I don't think that you should change your code to make it more testable. That's a fairly common viewpoint, as the goal of software is to solve some problem. Testing doesn't directly contribute to that, and making the function more complex is clearly worse at solving the problem.

That said, when it's hard to test your code, it'll be hard to change your code. Since change is pretty inevitable, making your code flexible will help you over the longer term. Sometimes that long term benefit isn't worth it, but it's rare.

As for this particular example, I'd be a little hesitant to inject the service. Having a simple abstraction for hasPermissions is useful. A better spot for the abstraction would be at your own consumer. It could take an interface or delegate/function-object that calls this static function in production. You could then mock that in your tests, without influencing all of the other consumers.

Refactoring this inflexible implementation can then happen over the longer term. (Ideally, removing the wildly inefficient linear search)

Improve this answer
2
  • why would you be hesitant to inject the service? is that because of having the change the code? Commented Apr 21, 2021 at 20:08
  • @Zort - because that ties the implementation of the function (PermissionsService) to its interface (the function signature). Commented Apr 21, 2021 at 22:34
2

Am I right to think that this is how tests are designs and our static method is tech debt because it wasn't designed with TDD in mind?

Technical debt doesn't (in its original form, anyway) mean bad code, or code that we don't like. It was specifically referring to incorrect alignment between the real domain and the design in code.

if we failed to make our program align with what we then understood to be the proper way to think about our financial objects, then we were gonna continually stumble over that disagreement and that would slow us down

This is just (take your pick):

  • bad code
  • code not designed with the affordances you need right now

other teams (who are also using the code) are objecting to this change.

And they are right to do so. Making a backwards incompatible change is an awful idea.

What you should instead do is make the change in a backwards compatible way, then (if necessary) deprecate the old way of doing things and negotiate a schedule to get everybody on board with the new approach.

The rigorous way to do it is to create a function with a new name that has the design you want, and then modify the old function to invoke the new function.

There are at least two ways you could do that here. One is, as you suggested, creating a function that takes the service as an argument

public static boolean hasPermissions(String username, int pageid) {
    PermissionsService s = new PermissionsService();
    return hasPermissionsV2(username, pageid, s);
}

public static boolean hasPermissionsV2(String username, int pageid, PermissionsService s) {
    int[] pages = s.getPages(username);
    for (int i = 0; i < pages.length; i++) {
        if (pages[i] == pageid)
           return true;
    }
    return false;
}

A different approach, which might be better for your situation, is to separate the code that manipulates values from the spooky side effects

public static boolean hasPermissions(String username, int pageid) {
    PermissionsService s = new PermissionsService();
    int[] pages = s.getPages(username);
    return hasPermissionsV3(pages, pageid);
}

public static boolean hasPermissionsV3(int [] pages, pageid) {
    for (int i = 0; i < pages.length; i++) {
        if (pages[i] == pageid)
           return true;
    }
    return false;
}

I suggest you review Spec-ulation; Rich Hickey's 2016 talk has a lot of interesting things to say about names, and the advantages of not changing the meanings of names.

Improve this answer

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.