prismicio-community / php-kit

Community maintained development kit for Prismic and the PHP language
https://prismic.io
Other
109 stars 83 forks source link

More efficient way to $doc->get($doc->getType() . '.field_name') #24

Closed tremby closed 10 years ago

tremby commented 10 years ago

I find that I'm doing a lot of $doc->get($doc->getType() . '.field_name') for various field names, and all the equivalents (`->getHtml' etc).

Writing it this way (with $doc->getType() in there) is good because I can more easily reuse code where the documents have different types but with common property names, but it is messy.

Are there cases when a document has properties directly from a different document type? If not, what's the point in namespacing on the document type at all?

If there is a good reason, then I wonder if it'd be possible to provide some method which is a shortcut to the above? Shame if that means duplicating it for all the different getters (getHtml, getImage, getText, ...), but it'd make my code a whole lot more concise.

tremby commented 10 years ago

I wondered just now if I could do $doc->get('my.field_name') a bit like in the API browser, but this doesn't return what I expect.

rudyrigot commented 10 years ago

Your thought makes sense, but the point of expliciting the type of your field's document (in plain text) is to make it less risky and more maintainable for you.

For instance, say you have a "product" type with a field "flavor", which is a group of links towards "flavor" documents; and say you have a "chef" document with a field "flavor", which is a structured text where each chef can discuss what their favorite flavors are. After you get both those fragments, you obviously can't at all apply the same methods to each one.

If you code $doc->get('flavor'), you may be mistakenly retrieving a structured text, while you were going for a group of links. Heavens will break loose later, when you attempt to use this variable in different ways. However, if you write $doc->get('product.flavor'), you know exactly which one you're getting; if you apply this to a $doc that is a "chef" document, you'll immediately get a nice error, to help you fix your mistake.

Another great reason to like that: document types in prismic.io are managed by document masks, which makes it very easy to change the type of a document type's fragment in a breeze. But once you've done that, you have to change your code to adapt it to this new fragment type; and when you have to chase all the occurrences of 'product.flavor' you'll be very happy to have written $doc->get('product.flavor') rather than $doc->get('flavor')!

Of course, you can bypass that security by using $doc->getType(), but you may have guessed this is not at all best practice. I do recommend that you don't, but the point of prismic.io is that you can extend things the way you want, so it's your call to choose what you want to make of it. :)

tremby commented 10 years ago

What if you have various document masks which have a "flavor" field, as you suggest, but they are the same type and are expected to contain the same kind of data? This seems like it would be a common use case, and in our project we have this all over the place. Is there no way to have these under a common namespace? Much like multiple classes implementing a particular interface.

If those common fields were inherited from an interface or what have you, they could be ->get('common_interface.flavor') and your chef can still have his ->get('flavor') or ->get('chef.flavor').

I am happy to forego the error protection you mention since I don't see that as much of a feature -- I'll get an error message whether it collects the wrong kind of data and attempts to do something impossible with it, or if it can't collect the data because the field doesn't exist. Same result, near enough. I'm using the Document extension in my comment over in the pull request mentioned above (slightly modified thanks to what @dohzya pointed out) and I'm content with that. I just feel that it'd be useful to others too, as an optional shortcut to $doc->get($doc->getType() . '.' . $fieldName).

rudyrigot commented 10 years ago

I definitely see your point, and your feedback has generated a lot of thoughts around the team. This is a matter of what's the best design between inciting the user to make sure they know what they're doing, and making it faster for them; and it's definitely not an obvious cursor to place.

For now, we're going to keep the kit's API design in place, and see how it goes. If we do end up preferring the way you're mentioning, it will be easier for backward compatibility to change from the way it is to the way you're saying, rather than the other way around.

tremby commented 10 years ago

Definitely.

I should clarify that I'm not saying the ->get() method's behaviour needs to change. I'm suggesting that adding shortcut methods like my ->getMy() would be helpful to have alongside ->get() etc.

rudyrigot commented 10 years ago

Ow, actually, I hadn't caught that, so thanks for specifying! It would be less invasive on the current kit's API indeed.

tremby commented 10 years ago

Well if you think it's worth considering, you should reopen the ticket. I think it's useful, but as I said I can get by with my extension class.

rudyrigot commented 10 years ago

This is an internal process thing: this won't especially happen from a ticket on one particular kit, because this is definitely a cross-kit feature: whenever we decide to do it, we'll do it for every single prismic.io kit at once, in every technology. It is noted on our side, don't worry. ;)