On Oct 7, 3:04 pm, Chris Meller <[email protected]> wrote: > On Oct 7, 2010, at 7:08 AM, rick c wrote: > > > To be clear, I had only considered deriving Tag from Term. not Tags > > from Vocabulary. While deriving Tags from ArrayObject may be poorly > > thought out, it acts more as a container for Tag objects than serving > > the other functions embodied in several of the static methods of > > Vocabulary. > > Why couldn't it be both? We're treating the new Term and Vocabulary classes > differently than we treated the old Tag and Tags classes, which essentially > did the same thing in a less extensible way. Right now Tags extends > ArrayObject and then implements a lot of methods that should be in the base > Vocabulary class. Like I said, all Vocabularies are going to have methods for > getting one / some / all of their terms. Why are we wrapping around the > Vocabulary methods in Tags when we could just extend Vocabulary and use them > directly? > > > > > Many of Vocabulary's staic methods, such as - get_all_object_terms(), > > get_all(), get_all_object_terms(), rename(), names() - make sense for > > a vocabulary in the abstract, but not for a specific vocabulary. > > Why don't they make sense for all vocabularies? Terms are linked to objects > and we want to get a list of all the terms for an object - that's the point > of having a vocabulary. Being able to rename terms is also a regular feature > you would expect. Why should I have to manually delete a term, create a new > one, and re-link it? Provide a rename function... problem solved. And if > something like renaming a term doesn't prove useful for your specific > Vocabulary (I'm not sure what that Vocabulary would be...) then you don't use > it... doesn't mean we shouldn't provide the method for the other 99% of > Vocabularies where it might be. >
They don't make sense for concrete vocabularies because these methods refer to vocabularies in aggregate, not to a single vocabulary. > > > Others, such as - get(), get_by_id(), rename(), while logical > > identifiers for methods in a specific vocabulary, are used for a > > different purpose in the Vocabulary class. In PHP, unless I'm > > mistaken, static members of a base class are not overridden in derived > > classes, so calling Tags::get_by_id(), for example, would result in > > Vocabulary::get_by_id() being called and incorrect results. > > Incorrect.http://paste.roguecoders.com/p/8df04ee938bdae392c6a99a2bf41e18d.html > > We would have to work on the logic for distinguishing whether we're getting a > Vocabulary or getting Terms for a Vocabulary, but I think that's an easy > problem to solve for the benefits it provides. > I'm glad to see I was wrong about overriding static methods. It's always good to learn something new and correct misinformation. Still, though, each class that wants to inherit from Vocabulary would need to override many of these methods to get the correct results. > Take a look at Posts vs. Post, for instance. You use Posts::get() for > fetching posts, regardless of the criteria you're fetching by. Why do we have > get_by_id, get_by_slug, and get_by_text for Tags? How about just Tags::get() > with an array of parameters? > I have no problem with a Tags::get() with parameters. The others are convenience methods incorporated because the original class used them, i.e. fear of breaking code. Even though people say that isn't something to be terribly concerned about at this stage, it is still something I avoid doing needlessly. > In the worst case scenario we could have a third class that represents > VocabularyTerms that Tags would extend, if we absolutely can't come up with > any way to logically indicate which methods in Vocabulary apply to the > vocabulary itself vs. its child terms. > > > The CRUD methods of the Vocabulary class make no sense at all to me in > > the context of a specifc vocabulary, unless they are methods used to > > manipulate terms within that vocabulary. Again, in the case of static > > methods, this would lead to incorrect results as the corresponding > > methods in the Vocabulary class would be the actual ones called. > > Take a look at Tags and Tag for CRUD functions. Where are the create and > delete methods? In Tag (Tags::delete() calls the delete method of the Tag > object it's passed, making it entirely pointless). You create and delete a > Term, not terms within a Vocabulary (Term::get()->delete() rather than > Vocabulary::get_term()->delete()). The CRUD methods in Vocabulary would apply > to the Vocabulary itself, which is very much true currently. Vocabulary > extends QueryRecord, which provides basic CRUD methods for vocabularies, not > terms within them. > Having Tags::delete() doesn't make a lot of sense. In a smarter container, CRUD methods for the contents might make sense. Vocabulary does participate in term creation and deletion with its add_term() and delete_term() methods. These don't matter much for a simple vocabulary like tags, but for more complex vocabularies, such as one that has a hierarchy, these are the safe methods to use to create and delete terms since the connections to the other terms in the vocabulary have to be maintained. If Tag/Tags, and other vocabularies can be created in a simpler, more efficient way with the changes you've talked about, I think it would be great. Rick -- To post to this group, send email to [email protected] To unsubscribe from this group, send email to [email protected] For more options, visit this group at http://groups.google.com/group/habari-dev
