couchDocument::record() does not return response object

[andreyvk]

Please, fix.

<?php
public function record() {
    foreach ( couchClient::$underscored_properties_to_remove_on_storage as $key ) {
        if ( property_exists($this->__couch_data->fields,$key) ) {
            unset( $this->__couch_data->fields->$key );
        }
    }
   $response = $this->__couch_data->client->storeDoc($this->__couch_data->fields);
   $this->__couch_data->fields->_id = $response->id;
   $this->__couch_data->fields->_rev = $response->rev;

   /****** SUPPOSED TO RETURN $response RIGHT HERE ********/
}

[ClemensSahs]

@andreyvk

Why we need return the response? If it works everything is fine, else you must handle a Exception.

[andreyvk]

No, it works fine, no doubt. It is just natural for the function to return response, since document was saved, right? storeDoc() returns a response. Why not record() ?

[ClemensSahs]

The document::record() is facade for client::storeDoc()

Because facade function doesn't response the same, this is a fundamental reason behind this pattern, yes they can but never must. ;)

In my mind $this is a better return value. Because I'm a big fan of "Fluent Interface"

// this is currently not possible
$doc->set('name','foo');
    ->set('email','foo@bar.local');
    ->record()
    ->storeAttachment(...)
    ->storeAttachment(...);

[andreyvk]

I guess this is just personal preference after all. How about fluent interface then? Any plans to roll that out? It's been a while!

There's also this issue with Lucene for Couch. There was a talk about it in issue #14 but then it kind of winded down. I had to add my own code to support Lucene from your lib and I am sure that I am not the only one. Might be useful to add it too.

[twiesenthal]

@ClemensSahs No offense,, but in my opinion this

// this is currently not possible
$doc->set('name','foo')
   ->set('email','foo@bar.local')
   ->record()
   ->storeAttachment(...)
   ->storeAttachment(...);

is just method chaining and not a fluent interface.

Usually a fluent interface should make your code more like a natural language -> i.e. having something like a grammar.

The example above might make you code faster because you don't have the overhead of typing $doc several times and you can take advantage of code completion, but for me it's just a train wreck.

Implementing a fluent interface for PHP-On-Couch would be a lot of work. It's not done with returning $this on every function call.

[ClemensSahs]

@twiesenthal Sry for this misunderstanding, yes the code i write is a "method chaining"...

I'm a fan from "fluent interface", but it will be a long way with the current API. So "methode chaining" is a little step in this direction. To use "fluent interface" we must break a lot of the current API.

"train wreck" xD nice comparative How ever I said that because we talk about the return value from record().

@andreyvk I'm sorry that #14 will not have a result, after we roll out PSR-0, we can push that Issues again so we avoid conflicts.

[dready92]

@andreyvk what is the use case of having $response as the return of this method ? If something breaks in the update of a document with POC, it should throw an exception. So IMHO the only valid return value for this method is $this.

[andreyvk]

I don't think it's about use cases. Just like I've mentioned before it just seems more unified this way with storeDoc(). Plus, I've never had a need for method chaining and I doubt that there will hardly be a need for that.

Just think about it. Storing multiple attachments - always in a loop. So no chaining here. Deleting attachments - a loop as well plus possibly an "if" to check if attachment exists before you delete it. The only method you do want to chain is probably set() and yet you hardly use that one as well because of __set that you have in place there.

I also prefer not to use the couchDocument() wrapper most of the time, because our documents' attributes are often complex / nested / arrays and __set() wont work well on that. But i do prefer to use it when it comes to finally storing documents and then adding a couple of attachments after that because it auto-updates _rev and _id for me. So I'd nearly always have something like this:

$client = new couchClient(...);

$doc = new stdClass();
$doc->attr1 = 'aaa';
$doc->attr2 = array(0, 1, 2);
$doc->attr3->sub_attr1 = 'bbb';

if(true) {
   $doc->attr2[1] = 10;
}

$cdoc = new couchDocument($client);
$cdoc->setAutocommit(false);
$cdoc->loadFromObject($doc); 
$cdoc->record();

foreach($attachments as $att_id) {
   $file = '/tmp/'.$att_id;
   if(!file_exists($file)) continue ;

   $content_type = get_content_type($file); //a custom function to get content type

   $cdoc->storeAttachment($file, $content_type, $att_id);
}

$doc = $cdoc->getFields();

echo(json_encode($doc));
exit();

So the bottom line is: if chaining isn't needed then why not return the response?

[dready92]

As I see it, the $response object won't give you any extra information : it will be returned only if the storeAttachment() call succeded. But if the storeAttachment() call succeded, $cdoc will be updated accordingly (with the good revision etc...). If it breaks, the storeAttachment call will not return a value, but throw an exception.

So to get errors you have to write it like this:

try {
    $cdoc->storeAttachment(...);
    echo "Attachment stored successfully !";
} catch ($e) {
    echo "There was an error storing the attechment. Code: ".$e->getCode().", message: ".$e->getMessage();
}

[andreyvk]

It might give me extra info if i am planning to return it back to the client as a json data. But anyways. This isnt going anywhere, so let's close the issue.