Re: [VuFind-Tech] Code Quality and VF2 refactoring

[David M. <ma...@ha...>]

At Wed, 18 Jul 2012 13:19:40 +0000,
Demian Katz wrote:
>
> > And all this is done for no particular reason: If the creator of a Results
> > object requires a Params in order to inject it via
> > __construct() and requires a Options in order to create the Params then it
> > already has access to the public methods of those objects.
>
> I do not contest that this is one of the ugliest parts of the new
> code, and it's a technical debt I've been meaning to repay...  but
> since I'm still in the midst of trying to get 2.0beta fully
> functional, I haven't had time to address it yet.
>
> And it's not done for no particular reason, it's done for two reasons:
>
> 1.) Sheer laziness -- putting in the __call() proxying got more of
> the existing SearchObject code working without modification after I
> refactored the single monolithic object into more manageable chunks.
> This could be easily remedied by adding getOptions() to the Params
> class and getParams() to the Results class and then making all the
> relevant calls explicit.
>
> 2.) View readability -- I like having a single results object in the
> view from which details can be easily extracted.  If you want to
> display "showing records 1-10 of 250" in the view, it's nice to be
> able to say "showing
> <?=$results->getStartRecord()?>-<?=$results->getEndRecord()?> of
> <?=$results->getRecordCount()?>" rather than "showing
> <?=$results->getParams()->getStartRecord()?>-<?=$results->getParams()->getEndRecord()?>
> of <?=$results->getRecordCount()?>."  Maybe this can be addressed
> with a view helper (though I'm not sure how, without that view
> helper doing ugly things), or maybe it's not important enough to
> worry about, and we should bite the bullet and put in some explicit
> getParams() calls where needed.
>

I do know that lazyness was partly the father of the __call(). My main
motivation for a rather harsh critic on __call() is that I saw you
started to use it in other places -- and __call() and the other
interceptor function are killer-features that should not be used
wisely because of the problems __call() creates maintenance-wise.

> In any case, cleaning up item #1 should be pretty straightforward
> and can be done gradually, leaving the __call() in for compatibility
> until it is no longer needed.  Item #2 might require a bit more
> brainstorming, but it's solvable as well.  Just a matter of finding
> time to do it.

Agreed.

Best,
  -- David
--
David Maus
Herzog August Bibliothek - D-38299 Wolfenbuettel
Phone: +49-5331-808-317
Email: ma...@ha...

PGP Key 0x0CC2E093512F7385
Fingerprint 1AD2 EE67 224F 18C5 EA55  98AD 0CC2 E093 512F 7385