jamulus icon indicating copy to clipboard operation
jamulus copied to clipboard

JSON-RPC - Change method names to remove redundancy

Open Rob-NY opened this issue 3 years ago • 5 comments

What is the current behaviour and why should it be changed?

Methods jamulusserver/setServerName and jamulusserver/getServerProfile are redundant in their naming. Propose changing to setName and getProfile, respectively.

Given the experimental state of JSON-RPC code, this change can be done without notice.

Describe possible approaches

Simple changes to method name literals in code and changes to associated (TODO) documentation.

Has this feature been discussed and generally agreed?

No

Rob-NY avatar Mar 10 '22 12:03 Rob-NY

In general, this is a good point in time to rethink this as JSON-RPC wasn't part of any release yet. I think we should be careful not to introduce opportunities for future ambiguities. The server has a rather full picture of the everything, so it might well be possible that we are talking about a client's profile at some point as well (and yet the method would not be named jamulusclient/...). A contrived example: If we ever had a method to rename clients (I'm not suggesting we do!), it might be called jamulusserver/setClientName. Having jamulusserver/setName would seem a bit strange in that case.

Personally, I'd tend to keep the names as in those two cases, but I'm not feeling strongly. Maybe others will comment as well.

hoffie avatar Mar 10 '22 20:03 hoffie

As we're thinking about this, perhaps we should revisit the overall naming. Having 'jamulus' in the method names is probably not necessary, and if we're comfortable with object->method style rpc2.0 naming perhaps consider:

jamulus/apiAuth (or just apiAuth) server/things_you_do_to_a_server client/* me/* connections/* session/*

and on the broadcast side, broadcast/* (as opposed to aligning with a client or server namespace)

So @hoffie, addressing your point, server/setName vs. client/setName vs. me/setName seems like a possibility. Each of these would be implemented in the rpc code module where it makes the most sense thus providing context and 'security'.

Clearly this isn't fully thought out. But to your point, if renaming is a consideration there are several paths to choose from.

Rob-NY avatar Mar 11 '22 13:03 Rob-NY

If we ever had a method to rename clients (I'm not suggesting we do!), it might be called jamulusserver/setClientName. Having jamulusserver/setName would seem a bit strange in that case.

I actually think it's fairly self-explanatory. "someObject->setName" will set the name of the object. "someObject->setThingName" will set the name of the thing on the object.

pljones avatar Mar 11 '22 16:03 pljones

Having 'jamulus' in the method names is probably not necessary

Agree.

jamulus/apiAuth could also be api/auth and we could then use other stuff on api/ (things we can't think of now.)

ann0see avatar Mar 11 '22 21:03 ann0see

jamulus/apiAuth could also be api/auth and we could then use other stuff on api/ (things we can't think of now.)

Actually, they should be jsonrpc if anything -- there could be different "general" APIs but this is specific to JSON RPC.

pljones avatar Mar 11 '22 21:03 pljones