Fix error when $terms[0] does not exist in open-graph

[mikeschinkel]

Summary

This PR can be summarized in the following changelog entry:

• Fixes a bug where a hard key, that might nog exists, is called. Props to mikeschinkel

Relevant technical choices:

• Use reset($terms)->name instead of $terms[0]->name because the first element may be $terms[1].
• Use !empty($terms) instead of $terms !== array() because it is clearer.

Test instructions

This PR can be tested by following these steps:

• Select more than more category.
• Do NOT set a Primary category
• Add 'get_the_categories' filter that unsets $term[0] (as a commercial theme a client was using does. It removes parent categories if child categories are selected.)

Quality assurance

• I have tested this code to the best of my abilities
• I have added unittests to verify the code works as intended

Fixes #

[andizer]

Thank you for your pull request 👍

I have one single comment. If you processed that I will take this pull request into our process to get it merged.

Thanks!

[andizer]

I would store the result of reset in a separate variable ($term). This makes it more clear that it's about a single term. Thus:

$term = reset( $terms ); 
$this->og_tag( 'article:section', $term->name );

Have you considered using array_shift instead of reset ? For me personally I needed to check what the return value of reset exactly is and it was the first time I see it being used in this context. It might be something to think of.

[mikeschinkel]

Hi @andizer - Sure, there are several ways to skin this cat, I just picked the most concise version, and intuitively chose reset() over array_shift() because reset()just changes the internal "current" pointer whereas array_shift() actually modifies the $terms array which requires both a new variable allocation and modification to an existing array and is thus a heavier approach.

I was curious so I benchmarked both and found reset() to be 19% faster than array_shift(), but doing it once the difference is so infinitesimal that it does not really matter which way it is done.

Same with copying to a variable. It is were me for shipping code where most people are users vs. readers of the code I would stick with reset() but since its your repo I'll make the requested update.

[mikeschinkel]

Done.

[andizer]

Thanks again! I have done a code review and an acceptance test. Besides of that I've added some unit tests to cover the situation.