Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

I have a web service (axis2) and the server performs 4 primary operations: createX, removeX, getX and updateX

Where X is represented by a combination of 4 values: id1, id2, id3 and id4

So, to simplify developer life, we created 3 extra helper functions:

removeXForId, getXForId and updateXForId where we represent the set of 4 values as 1 Identifier so that the developers who are calling us do not need to pass all those 4 values again and again and just store the unique identifier on their end.

But, we cannot also completely remove removeX, getX and updateX operations completely abd just rely on the unique identifier as some use cases need to pass all 4 set of values.

So, now we have around 7 operations where half (3) of them are duplicate operations with different input values.

Operations:

/createX
/getX {input: id1, id2, id3, id4}
/getXForId {input: id1234}
/removeX
/removeXForId
/updateX
/updateXForId

Is this a good practice?

UPDATE:

getX is exactly the same as getXForId in terms of behavior except the input arguments.

getX takes in 4 inputs: val1, val2, val3 and val4. But getXForId takes in just 1 ID which represents (val1, val2, val3 and val4)

val1, val2, val3 and val4 are some how related to each other, they in combination represent a unique link to a product in our system. So we tried to club them together.

share|improve this question
    
The API should say what it means. If I want to update "id1,id2,id3,id4" I should be able to say just that. Messing around with abstract tokens is not going to save your client programmer any time and just cause confusion. –  James Anderson May 23 '12 at 1:47

3 Answers 3

up vote 4 down vote accepted

It would make more sense to me to organize the API around resources and use the verbs (GET, PUT, POST, DELETE) that come with the http protocol, in short: Make it more RESTful, that is surely considered good practice.

This is a good succinct talk on API design that also analyzes popular APIs such as facebook's or twitter's.

I am lost on the difference between your /getX and /getXForId so I can't really give you an example of how I would change your API.

Edit

I tried to model the API in Sinatra. It is pretty self explanatory and straight forward. (I saw you are using axis but it's been a while since I did serious Java). If you are serious about RESTful then you probably want to drop the approach with the four ids. However the approach works:

require "sinatra"
require "sinatra/reloader"

def all_ids_given?(ids_given, ids_needed)
  ids_needed.reduce(true){|memo, id| memo &= ids_given.include? id}
end

#Your old /getXForId
get '/X/:combined_id' do |id|
  "Getting X with combined id #{id}\r\n"
end

#Your old /createX
post '/X' do
  post_params = request.body.read
  "Creating a new X with #{post_params} \r\n"
end

#Your old /updateXForId
put '/X/:combined_id' do |id|
  put_params = request.body.read
  "Updating X with combined id #{id}: New values are #{put_params}\r\n"
end

#Your old /removeXForId
delete '/X/:combined_id' do |id|
  "Deleting X with combined id #{id}\r\n"
end

#Your old /getX
get '/X' do
  if all_ids_given?(params.keys, ["id1","id2","id3","id4"])
    "Getting X with the ids #{params}\r\n"
  else
    "Error: Need arguments for id1, id2, id3 and id4\r\n" 
  end
end

#Your old /updateX
put '/X' do 
  if all_ids_given?(params.keys, ["id1","id2","id3","id4"])
    put_params = request.body.read
    "Updating X with the ids #{params}: New values are #{put_params}\r\n"
  else
    "Error: Need arguments for id1, id2, id3 and id4\r\n" 
  end
end

#Your old /removeX
delete '/X' do 
  if all_ids_given?(params.keys, ["id1","id2","id3","id4"])
    "Deleting X with combined id #{params}\r\n"
  else
    "Error: Need arguments for id1, id2, id3 and id4\r\n" 
  end
end

#Extensibility
get '/X/:combined_id/Y' do |id|
  "Getting all Ys for the X with combined id #{id}\r\n"
end

Testing the API with curl:

base_url="http://localhost:4567/"

echo "Getting a X with a combined id"
echo -n "-> "; curl ${base_url}X/id1234
echo "Creating a new X with a list of properties"
echo -n "-> "; curl --data "property_1=test"  ${base_url}X
echo "Updating an existing X with a combined id"
echo -n "-> "; curl -X PUT --data "property_1=test&property_2=another_test"  ${base_url}X/id1234
echo "Deleting an existing X with a combined id"
echo -n "-> "; curl -X DELETE ${base_url}X/id1234

echo "Getting a X with 4 ids"
echo -n "-> "; curl ${base_url}"X?id1=1&id2=2&id3=3&id4=4"
echo "Getting a X with 4 ids, but supplying only 3"
echo -n "-> "; curl ${base_url}"X?id1=1&id3=3&id4=4"
echo "Updating a X with 4 ids"
echo -n "-> "; curl -X PUT --data "property_1=test&property_2=another_test"  ${base_url}"X?id1=1&id2=2&id3=3&id4=4"
echo "Updating a X with 4 ids"
echo -n "-> "; curl -X DELETE ${base_url}"X?id1=1&id2=2&id3=3&id4=4"

echo "\r\nTesting the extensibility"
echo "Getting all Ys for a single X (Unrelated example: /post/2/comments would give all comments to a single post with the id 2)"
echo -n "-> "; curl ${base_url}X/id1234/Y
echo "This won't work for the 4 ids"
echo -n "-> "; curl ${base_url}"X?id1=1&id2=2&id3=3&id4=4/Y"

The script returns the following:

Getting a X with a combined id
-> Getting X with combined id id1234
Creating a new X with a list of properties
-> Creating a new X with property_1=test 
Updating an existing X with a combined id
-> Updating X with combined id id1234: New values are property_1=test&property_2=another_test
Deleting an existing X with a combined id
-> Deleting X with combined id id1234
Getting a X with 4 ids
-> Getting X with the ids {"id1"=>"1", "id2"=>"2", "id3"=>"3", "id4"=>"4"}
Getting a X with 4 ids, but supplying only 3
-> Error: Need arguments for id1, id2, id3 and id4
Updating a X with 4 ids
-> Updating X with the ids {"id1"=>"1", "id2"=>"2", "id3"=>"3", "id4"=>"4", "property_1"=>"test", "property_2"=>"another_test"}: New values are property_1=test&property_2=another_test
Updating a X with 4 ids
-> Deleting X with combined id {"id1"=>"1", "id2"=>"2", "id3"=>"3", "id4"=>"4"}

Testing the extensibility
Getting all Ys for a single X (Unrelated example: /post/2/comments would give all comments to a single post with the id 2)
-> Getting all Ys for the X with combined id id1234
This won't work for the 4 ids
-> Getting X with the ids {"id1"=>"1", "id2"=>"2", "id3"=>"3", "id4"=>"4/Y"}

What you can see is that nesting resources and the four parameter approach does not work well together. Maybe one could model it differently and do something like /X/:id1/:id2/:id3/:id4.

share|improve this answer
    
updated my question, does that help? –  zengr May 22 '12 at 21:17

What problem does your "combined identifier" operations solve, anyway? You say it is because you don't want to send all of the identifiers separately, yet I imagine you have to send them all regardless of which operation you use. So, unless there is some other problem the "extra" operations are solving, the simple answer is to just not have them.

If you really do want these "extra" operations because it makes the client easier to implement, you could always implement them in the client code and use them when you need them. For example, you could have a function that parses the combined identifier, and then you can use it whenever you need to make an API call. This way the API stays clean and the extra operations are scoped properly.

share|improve this answer
1  
I totally agree with your point. To be honest, it's not my design and I am working on it as team member. Asked this question to have rational justification of this design. But I think, its a bad approach. –  zengr May 23 '12 at 2:42

This sounds terrible, storing a unique id as state on the server won't scale in the long run.

A Stateless REST based system would be a infinitely better choice.

Passing all the values every time is what will make it scale. Trying to save a few characters a developer has to type to send arguments will cost you 1000 X more effort trying to track and scale the state on the server side.

Abandon what you are doing before it is too late to turn back and you and your successors have to deal with this bad design decision for the next 10+ years.

Your clarification in your update is just as bad but in a different way, it completely violates the DRY Principle.

share|improve this answer
    
I have updated my question, I hope you understand what I meant. Thanks! –  zengr May 22 '12 at 21:19
    
your clarification makes it even worse ... but in a different way –  Jarrod Roberson May 22 '12 at 21:38
    
makes the question worse or the design worse? –  zengr May 22 '12 at 22:13
2  
I Downvoted for two reasons. The "REST" is just plain irrelevant here. His scheme as described could easily be implemented using a REST API. Also he does nor repeat himself rather he adds another (pointless) layer of functionality so the DRY comment alos misses the point. –  James Anderson May 23 '12 at 1:51
    
What is in the question is clearly RPC style calls, if they they are REST you don't know what REST really is! And having 2 methods that do exactly the same thing with different names and the same args just concatenated together violates DRY and SOLID at the same time! –  Jarrod Roberson May 23 '12 at 18:56

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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