Where communities thrive


  • Join over 1.5M+ people
  • Join over 100K+ communities
  • Free without limits
  • Create your own community
People
Repo info
Activity
PG Lewis
@pglewis
and show_form, of course
Justin Sternberg
@jtsternberg
does that conflict with your stuff?
basically, if they add fields an an old-school array, it loops through them and adds them the right way
PG Lewis
@pglewis
probably, but it's not exactly a giant changeset
I'm trying to merge back up from your trunk, to ours, and up to my branch
test changes I put in our trunk and submit the PR from there
so if you make changes I'll end up merging them in on my own, if that means refactoring it for working the "new way"
Justin Sternberg
@jtsternberg
that's a lot of setters/getters. Is there any reason there should be a method for all of them, vs one setter/getter?
PG Lewis
@pglewis
my add_fields isn't even in cmb2
because it's easier to manage in the long run, IME
most of the tedium is in creating them, and phpStorm will generate skeletons for every class property I highlight
Justin Sternberg
@jtsternberg
how/why? (not being argumentative, just want to understand)
PG Lewis
@pglewis
if you have all your logic for all your classes properties in one method, that method can grow pretty big
and 9 times out of ten it's just if/elseif or switch/case anyway
also benefits from doing it that way are autocompletion and phpDoc with a keystroke
having those allows you to change the whole underlying implementation later if you want to
the getters and setters are what are exposed and used on the consuming side, and hide the details so you're more free to change them in the future
though a single getter/setter does that too
Justin Sternberg
@jtsternberg
yah, that's my main question. Why not one (or fewer)
PG Lewis
@pglewis
let me turn it on it's head for a moment, what's the benefit of not having separate ones?
PG Lewis
@pglewis
with individual getters and setters you can phpDoc the return values properly for each one
Justin Sternberg
@jtsternberg
yah, that's true. Curious, how does this handle custom metabox properties?
PG Lewis
@pglewis
this in turn means my IDE knows when I call get_field_objects() and foreach through it, each element is a Field object
and i get autocomplete on the field object as a freebie
Justin Sternberg
@jtsternberg
because users can (and do) add arbitrary data to the cmb array and then pull it out elsewhere
Justin Sternberg
@jtsternberg
since we're renaming things, I think we definitely should rename the cmb_id property/methods to just id
just kludged to add it to it's own free form array that I tack back on with array_merge()
just there for back compat or I wouldn't have thought of it
Justin Sternberg
@jtsternberg
how about back-compat for the prop method?
PG Lewis
@pglewis
passes all tests, including some I added specifically for prop
Justin Sternberg
@jtsternberg
since that's how the arbitrary values are retrieved currently
ah ok, you still have the prop method. should have looked
PG Lewis
@pglewis
reworked to use the "new way"
everything internally in cmb2 should be using the getters/setters now
and helper functions too
outside code doesn't yet, but the back compat bits should be catching nearly all that
Justin Sternberg
@jtsternberg
yah, makes sense. so my biggest opposition to the individual setters/getters is the non-DRY redundancy throughout this file. Just eats at me. :) not necessarily a good enough argument to remove, but...
PG Lewis
@pglewis
so, ultimately, it's "doable"
explicit getters and setters are the one time I come off my "less code is always better" stump
or one of the rare times
but in the end, it's usually not much less code overall and the IDE can generate the skeletons
Justin Sternberg
@jtsternberg
at least do this?
        case 'id':
        case 'title':
        case 'type':
        case 'object_types':
        case 'context':
        case 'priority':
        case 'show_names':
        case 'show_on_cb':
        case 'show_on':
        case 'cmb_styles':
        case 'enqueue_js':
        case 'fields':
        case 'hookup':
        case 'save_fields':
        case 'closed':
        case 'new_user_section':
            return call_user_func( array( $this, 'get_'. $property ) );
:D
PG Lewis
@pglewis
a single getter/setter approach still has to have all your logic
lol