0

I've a service called Claims-Service which has following signature:

IClaimsService { EnsureClaim(claimType, userId) }

It queries database to see if the claim is present in any of my roles or not.

Based on my current requirements, I don't have a scenario which requires a user to have more than one claim to do something, but I think I'm going to need it in near future. (I'm in the very beginning of the project)

Is it ok to add following method to my service or not?

EnsureClaims( claimTypes, userId )

So I can pass array of claim types to it.

What pros and cons do you see if I develop the second method in my service?

Note that EnsureClaim and EnsureClaims need their own queries and there won't be any type of code re use. So I'm talking about developing new codes, automated tests & code review. And I'm not talking about huge amount of new codes.

Yaser Moradi
  • 109
  • 3
  • I can no longer post an answer here, but I'd like to offer a different perspective than the current answers (as I find them to be obtuse and beside the point). We get it. Adding code requires testing, dev time, yada yada... As if we didn't already know this. The question you are asking, though, has much less to do about "future-proofing" than it does about boundaries. Specifically, consistency boundaries. This is about *design*! The two options you present imply a different granularity regarding the transaction. If the *design* of your system mandates that each time `EnsureClaim` is invoked... – user3347715 Jun 03 '19 at 15:14
  • ... a single transaction needs to be opened and either committed or rejected, then adding another method called `EnsureClaims` makes no sense. What would that method do? Internally loop over each item, open a *new* transaction, and execute `EnsureClaim`? Or would it attempt to ensure all claims provided in one transaction? It's unclear given the lack of requirements you list. The point I'm making is that the choice between adding the method to your interface vs simply asking the caller to write a `for` loop is **not** a question of code maintenance. It is a question of *design*. – user3347715 Jun 03 '19 at 15:19
  • @king-side-slide As I mentioned in my question, there is no any type of code reuse between those two methods. So I'm not talking about for each in EnsureClaims method which calls EnsureClaim method in each loop. – Yaser Moradi Jun 03 '19 at 16:55
  • My point is not about code-reuse. That is beside the point. I'm asking you to think about the *design* - something where the lack of code re-use it telling. Why is there no re-use? What is it about the *design* of your system makes this impossible/impractical. Why is it that ensuring multiple claims is not *conceptually* the same as simply ensuring a single claim in a loop? Why is having a method `EnsureClaim` preferable to only `EnsureClaims` where the caller can pass an array with a single value? – user3347715 Jun 03 '19 at 17:15
  • To be explicit (where I have only been implicit thus far), there is something wrong with your design where *both* of these methods could exist! One of them should not make sense. Either this business use-case operates at the level of a single claim, or it does not. Either you expect some higher-level procedure (the client, or an API) to loop or you do not. Good (simple) design is not achieved by adding "short-cuts", rather, it is achieved by *limiting* the possible operations to only those necessary to carryout your given use-case. – user3347715 Jun 03 '19 at 17:16
  • There is nothing wrong (and it is often preferable) to put the onus on the caller (even if that is you). – user3347715 Jun 03 '19 at 17:16
  • It's just an example. What would you expect if you say, hi everybody, I've developed method EnsureClaim and I'm going to call it in a loop in method EnsureClaims. is it good to develop EnsureClaims or not and everybody answers yes, write it! I'm talking about a scenario when you've developed method A, and you're thinking about adding another method in that service that you might need it in near future. Do you write it at that time or not? – Yaser Moradi Jun 04 '19 at 05:34
  • What I'm asking you to do is **not** simply consider whether or not you should write a method. I'm asking that you allow the *behavior* of your system to inform the design. Specifically, *you* need to carefully consider the boundaries of your components (in regard to consistency/transactions). Is `EnsureClaim` bound to a single transaction? I do not have a mental model of your system so I cannot give you *the* answer, but adding a method to save the caller a for loop is a waste of time. It doesn't make your system any more cohesive. I would consider such a method "cruft" or technical debt. – user3347715 Jun 04 '19 at 15:26

2 Answers2

1

There is much to consider when making a decision like this. Some of the questions you need to get answers for include:

  • Can you afford to take the time to develop and test the currently unused code?
  • Will it add additional requirements (like additional memory)?
  • How likely is it that you'll actually need this feature in the near future?
  • Will this additional code need to be rewritten for whatever version it is used in because something else changed?
  • Will the tests need to be updated as the product evolves between when you add the feature and when it is actually needed?
  • Will the new code introduce any additional points that can be exploited (security vulnerabilities)?
  • Are there be development assets available now that can be used to add the feature that will not be available in the future?

If you do eventually add the feature, you'll still need to go thru the testing phase.

This usually resolves itself to "No, don't add the second service now."

1201ProgramAlarm
  • 1,529
  • 2
  • 14
  • 21
1

This seems like something you can add fairly easily at any point (especially if this service if new, in terms of its life-cycle as a product, since in that case the cost of rewriting whole chunks of code is probably low).

While it is OK to add support for anticipated features, especially where they can be expected with a degree of certainty and are well understood (assuming you otherwise exercise YAGNI-like restraint where appropriate), since you say there's no code reuse, I would leave this for later. Even though it's not a huge amount of new code, you may end up needlessly complicating things, or you may make some wrong assumptions which could get in your way later.

A better thing to focus on instead is to make sure that the code that uses the claims service (and other things related to claims) doesn't depend on the fact that the code behind it checks for a single claim (i.e, is not written in a way that relies on that), so that you can introduce the change later easily with minimal consequences. So, check for that, and refactor if necessary.

Filip Milovanović
  • 8,428
  • 1
  • 13
  • 20