should we introduce civicrm_api3

[Eileen]

We talked about this on a ticket. Basically we agreed that there were advantages of a function

civicrm_api3($entity, $action &$params){
  $params['version'] = 3;
  return civicrm_api($entity, $action $params);
}

The advantages being
1) having a different fn name makes it easier to grep for &
2) it saves setting the version.
3) it saves the point of having version - allowing the api wrapper to continue supporting api3 while we implement api 4

Obviously introducing this is a very quick task. Promoting it / updating the examples would be a little slower.

[Erik Hommel]

Makes sense, I pledge to update the developer guide when we do have civicrm_api3.

[totten]

civicrm_api3 sounds more pleasant/usable.

I was a little concerned that the name might conflict with "civicrm_api3" from api/class.api.php, but PHP doesn't seem to mind if a function and a class share the same name.

[xavier]

Are we pushing it too much if we introduce a new feature: civicrm_api3 does raise an exception on error, instead of only is_error

(as in, by default, it crashes if you don't test is_error)

X+

[Eileen]

Hmm  - I'm not sold on that - but perhaps others are.

[Erik Hommel]

I need a little more explanation Xavier?

[xavier]

So a common pattern is I want to fetch an id of something:

$result = civicrm_api("Whatever","get",params...);
$myid=$result["values"][0][id];
//a long while after, do something else with myid

the issue is that it doesn't test on is_error and if there is a pb somewhere, $myid is going to be empty and it happily going to make a lot of weird errors hard to catch down the line. Another common one is to create something and assume it was created.

so the proper way is
$result = civicrm_api
if ($result["is_error") {// needed, but not mandatory
  //fix or do something
}

by using exceptions on errors, you'd do the same

try {
  $result = civicrm_api
} catch {
  // fix or do something
}

So the code isn't that much longer, the big benefit is that if you don't do the try catch

$result = civicrm_api("Whatever","get",params...);
if will stop with a clear error message.

So in short: force us lazy developers to be less sloppy, one fatal at a time

X+

[Erik Hommel]

Yep, I get that...I always do the
if ( $result['is_error'])

That bit is clear to me, but that means that as of the next version, we do not provide the is_error anymore?

[xavier]

...I always do the
if ( $result['is_error'])

Always? Us mere mortals tend to do it, the best of our kind might reach the almost always, but, according to the legend, even Tim has not tested it at least once.

Anyway, wasn't to remove, but on the top of. Or we might introduce a options.error_type=attribute|exception|both (the later being the default)

[Coleman Watts]

So what about the other bindings?
javascript: CRM.api3('foo', 'get')
smarty: {crmAPI3 entity='foo' action='get'}
rest: no change?

[xavier]

That's a really good question. would be more coherent... but even more places to change the doc...

I think version was a mandatory param only for php anyway.

Plan B: we remove version as a mandatory field and default to version 3 we keep civicrm_api, CRM.api and {crmAPI} as v3 only and if/when api 4 is there, we introduce civicrm_api4 CRM.api4 and crmAPI4 at that time?


X+

[Coleman Watts]

I like plan b, at least for the alternate bindings. For php I see no reason not to introduce civicrm_api3 in 4.4 as a thin wrapper around civicrm_api. People don't have to use it if they don't want to, but it will provide some consistency going forward with api4.
Not sure whether that's the time to introduce exceptions or wait for api4. I could be convinced either way.

[nicolas]

I like the idea of raising an exception over returning is_error - much cleaner. I also agree there is no need to change the other bindings.

On updating the documentation, do we need to replace/obsolete the old method or just add this new one? I would rather consider this an addition only and keep both options open for developers in the future (ie. api v4 called either through civicrm_api with 'version' => 4, or through civicrm_api4).

[Erik Hommel]

Xavier, the only thing I always do is forget something :-)
Happy with the exception and happy with plan b!

[Eileen]

I'm confused (permanently)

Is plan b that we leave civicrm_api as it is but make 'version' => 3 a default rather than required.

We also introduce civicrm_api3 which wraps civicrm_api but throws an exception