Opened 15 years ago
Closed 14 years ago
#12526 closed enhancement (fixed)
Introduce has_term() to the tax API
Reported by: | ptahdunbar | Owned by: | |
---|---|---|---|
Milestone: | 3.1 | Priority: | normal |
Severity: | normal | Version: | 3.0 |
Component: | Taxonomy | Keywords: | has-patch needs-refresh |
Focuses: | Cc: |
Description
Simple patch that just extends has_tag() into has_taxonomy(). Tested, and works like charm.
Attachments (4)
Change History (25)
@
15 years ago
introduces has_term, deprecates in_category in favor of has_category, and has_category and has_tag are wrappers for has_term
#2
@
15 years ago
- Keywords dev-feedback added
- Summary changed from Introduce has_taxonomy() to the tax API to Introduce has_term() to the tax API
#8
@
14 years ago
Patch needs refresh. Also, we can get rid of the $GLOBALSpost? stuff and just use get_post().
#11
@
14 years ago
- Resolution fixed deleted
- Status changed from closed to reopened
Please improve the function argument order, first $taxonomy, then $term. Looks more natural to me.
Example:
function has_term( $term = '', $taxonomy = '', $post = null ) {
to:
function has_term( $taxonomy = '', $term = '', $post = null ) { }
Next to that, those arguments are not optional, in this implementation they are. That's probably a mistake.
I don't see any necessity to introduce optional arguments here. The post->id can be generated in the deprecated function.
I check if I can make a useful patch, reopening so far.
#12
@
14 years ago
Functions that are easy to read (e.g. that consists only of a function call to has_term()), when the make use of the @use docblock that states this simple fact, is only adding redundant information and instead of helping, makes it harder to read and understand the function.
#13
@
14 years ago
That would be inconsistent with all the rest of the tax api which has $term first and $taxonomy second.
#14
@
14 years ago
Replying to scribu:
That would be inconsistent with all the rest of the tax api which has $term
first and $taxonomy second.
Oh that's not true. For example is_object_in_term() has $taxnomy first and then $term. All the functions that make use of has_term not even have the taxonomy name in their function name.
#15
@
14 years ago
Made a patch that puts the arguments into the expected order and removes their optional status (at least for the first two) as it makes no sense to run a check on a NULL taxononmy or look for the NULL term.
Additionally I tried to make the docblocks less complicate. It's far away from perfect, just one little step, I'm sure everybody else will find additional stuff that can be improved with the docblocks. Looks like a lot of redundant information in there.
There were some minor issues that looked like being created by doing copy & paste unreviewed.
#16
@
14 years ago
All the functions that make use of has_term not even have the taxonomy name in their function name.
They're not relevant to the discussion precisely because they don't have a $taxonomy parameter.
Oh that's not true. For example is_object_in_term() has $taxnomy first and then $term.
Ok, so you found a single counter-example. That doesn't validate adding yet another inconsistency.
Made a patch that puts the arguments into the expected order and removes their optional status (at least for the first two) as it makes no sense to run a check on a NULL taxononmy or look for the NULL term.
NULL taxonomy no, but NULL term yes: check if the post has any terms in the given taxonomy.
#17
@
14 years ago
Well the natural order is that you start with the more broad one (e.g. taxnomony) and then you locate the concrete in there (e.g. term). So my first argument is the expected order of arguments. Remember, this is introducing a new function.
Next to that I just prooved you wrong. There are more than that one function where this is the case.
NULL term as a joker. Awful if you ask me. I suggest to add an aditional function: has_taxonomy() and then to check for taxnonomy only - not for a term (as the function name suggests).
Naming should be precise. At least we should try hard if we introduce something new :)
#18
@
14 years ago
- Resolution set to fixed
- Status changed from reopened to closed
Our API is quite consistent with term then tax.
Closing as fixed.
#20
@
14 years ago
- Resolution fixed deleted
- Status changed from closed to reopened
Last commit did contain misleading information in the docblocks at least. Just pick the stuff you're ok with from the patch and drop the rest nacin. But even try to iterate when there's some feedback for the good of us all.
Missing a return in there.
There might be a better location for the has_taxonomy() function too, But i'm not too sure where all the other taxonomy functions are stored right now..