4

Say i have a Rest API that has a POST and GET method.

If i want to overwrite a resource in the API i can call the GET method to get the original item and then call the POST method to replace that resource after i am done modifying it.

Should i add the update functionality to the client class like in the first approach or should i make a new class and add the update functionality there and call the client class externally like in the second approach

First approach

class BookApiClient{

 Client restClient = ClientBuilder.newClient();

 public Book getBook(String id){

 // code to get book using rest client
 }

 public void postBook(){

 //code to post a book

 }


 public void updateBookTitle(String id, String newTitle){

  Book book = getBook(id);

  book.setTitle(newTitle);

  postBook(book);

 }

}

Second approach

class BookUpdater{

 BookApiClient bookService;

 public BookUpdater(BookApiClient bookService){

   this.bookService = bookService;

 }

  public void updateBookTitle(String id, String newTitle){

  Book book = bookService.getBook(id);

  book.setTitle(newTitle);

  bookService.postBook(book);

 }

}

Would it be a violation of single responsibility principle to add the updateBookTitle method to the client class or have that as a different like in the second approach

Claudiga
  • 149
  • 2
  • 1
    Possible duplicate of [How to determine if a class meets the single responsibility principle?](https://softwareengineering.stackexchange.com/questions/154723/how-to-determine-if-a-class-meets-the-single-responsibility-principle) – gnat Aug 08 '18 at 16:47
  • Another reflection on a dubious "principle". If it was truly a principle, you would not need to ask the question -- the SRP has always been ambiguous. – Frank Hileman Aug 08 '18 at 16:49
  • 1
    The 'single responsibility' of an API is to provide a programmatic interface to your application. Seems to me like you can have as many individual functions in there as you need without 'violating' the dubious SRP. Do you commonly need to update only the book title? Then put it in without worry, I say. – Eric King Aug 08 '18 at 17:35

2 Answers2

6

The first approach seems fine and consistent. It doesn't contradict SRP: The single responsibility principle is not about what the class does but about its reasons to change, as explained by Uncle Bob (the "inventor" of SRP) in this excellent article.

Furthermore, your REST API could offer a proper PUT instead of POST for a full replace (and PATCH for a partial update). Using the first approach would therefore have the advantage of hiding the details of the API, and offer a simpler interface for handling books.

The BookAPIClient would then act as a remote facade. The single reason to change it would be the evolution of the book API.

Christophe
  • 74,672
  • 10
  • 115
  • 187
-1

Yes.

The client should only be concerned with communication with the server. It should only have methods which the server offers. It's only, 'reason to change' if you will, should be that the server has changed.

UpdateTitle adds extra business logic which should be in a different layer of your application.

What if, for example, when updating a title you also want to write an audit log to your auditClient? do you now inject audit client into BookClient just in case that one method is called? Where would the madness end?

Furthermore your GET book request should use a POST incase the id string exceeds the max URL length!!!

Ewan
  • 70,664
  • 5
  • 76
  • 161
  • This is not an answer to the question. I agree that a BLL abstraction would be useful here but it's simply not the focus of the question. You're arguing against the method body, but the question is about adding a new method to an API (regardless of its body). Secondly, the suggestion that all GET methods should be rewritten to POST methods is nonsensical. GET and POST have very specific intentions ([more info here](https://stormpath.com/blog/put-or-post)) that go well beyond the URL length. I see no justifiable reason to expect OP's code to expand beyond the URL limit. – Flater Aug 09 '18 at 08:23
  • @Flater maybe im being slightly tongue in cheek with the POST comment, but I think you have misread the question. the method isnt being added to the api. Its being added to the client – Ewan Aug 09 '18 at 08:27
  • I did in fact misspeak, it's about adding a method to the class that calls the API, but the rest of the objection remains the same. The new method does not violate SRP, it merely provides an altered way to trigger a `postBook` method. While this isn't literally an overloaded method, the extra method serves the same purpose - an extended option. **There is no rule/guideline that stipulates that `BookApiClient` is not allowed to have more methods than the Book API itself.** As long as the methods are intended to interact with the API, they fit in the `BookApiClient` class. – Flater Aug 09 '18 at 08:41
  • @Flater we disagree then. I say the Client has the responsibility to expose the APIs methods in the Calling codes language. Anything extra violates SRP – Ewan Aug 09 '18 at 09:04
  • 1
    That would be the same as arguing that a Repository is only allowed to expose EF methods but is not allowed to create custom methods which are no more than a _composition of EF methods_. Such a strict regime effectively negates the benefit of having an additional layer, as you are forcing it to mirror its predecessor and disallow it to add any functionality. – Flater Aug 09 '18 at 09:18
  • Not at all, the analogous example would be the repo can only expose methods supported by its datasource. ie the tables would have to exist – Ewan Aug 09 '18 at 09:23
  • It makes no sense. Because if you apply that logic, then _every_ layer becomes a mirror of its predecessor, which means that you completely lose out on layer-specific implementations. The BLL would be a dumb wrapper around the DAL. The API would be a dumb wrapper about the BLL. This completely removes the option of implementing _handling logic_, and instead forces the consumer (web client) to utilize the methods based on how the data source (many layers down) operates, which defeats the inherent benefit of having layer separation. – Flater Aug 09 '18 at 09:30
  • you are introducing this 'predecessor' idea, not me. I'm saying in the case of the Client-Server relationship the Client's methods should mirror the Servers methods. Its single responsibility is how to talk to the server and to deserialise the response. – Ewan Aug 09 '18 at 10:17
  • The predecessor is simply "the layer below", sorry if that was not clear. I'm not introducing a new idea, layering inherently implies that the layer is placed on top of something else. – Flater Aug 09 '18 at 10:43
  • in which case the server is not the 'predecessor' of the client as its not part of the same application – Ewan Aug 09 '18 at 10:46