From: <Pet...@pa...> - 2010-03-03 02:05:43
|
I've been doing some testing on the record driver refactoring. First, this is a really fantastic enhancement to vufind. I was able to extract some records from a DB/Text database and insert them into the solr index. I then created a DbtextRecord driver which extended the IndexRecord - all very elegant and required minimal code. The only issue I had was theming the results. Many of the IndexRecord functions return a path to a template in the RecordDrivers directory for the theme. At first I thought I could simple add another directory for my own Dbtext RecordDriver, but then I worked out that I would have to override all the RecordDriver functions that returned template paths. Fortunately, that wasn't too hard to do by doing something like e.g. for the getCoreMetatdata function in DbtextRecord.php: public function getCoreMetadata(){ parent::getCoreMetadata(); return 'RecordDrivers/Dbtext/core.tpl'; } I don't know the code well enough to know whether it is feasible to decouple the template paths from the RecordDrivers (or even if it is worth it). As I said, this is a great improvement to vufind and has got me really excited at the possibilities of getting more of our resources exposed by vufind. Well done! Peter -- Peter Neish Systems Officer Victorian Parliamentary Library Ph: 03 9651 8638 pet...@pa... <!DOCTYPE html PUBLIC "-//W3C//DTD HTML 4.01//EN" "http://www.w3.org/TR/html4/strict.dtd"> <html> <head> <meta content="text/html; charset=ISO-8859-1" http-equiv="content-type"> <title>Disclaimer</title> </head> <body> <big><big style="color: rgb(0, 0, 153);"><br> Parliament of Victoria</big></big><br> <span style="color: rgb(204, 0, 0); text-decoration: underline;">Important Disclaimer Notice:</span><br> <br> <span style="color: rgb(153, 153, 153);">The information contained in this email including any attachments, may be</span><br style="color: rgb(153, 153, 153);"> <span style="color: rgb(153, 153, 153);">confidential and/or privileged. If you are not the intended recipient, please</span><br style="color: rgb(153, 153, 153);"> <span style="color: rgb(153, 153, 153);">notify the sender and delete it from your system. Any unauthorised</span><br style="color: rgb(153, 153, 153);"> <span style="color: rgb(153, 153, 153);">disclosure, copying or dissemination of all or part of this email, including</span><br style="color: rgb(153, 153, 153);"> <span style="color: rgb(153, 153, 153);">any attachments, is not permitted. This email, including any attachments, should</span><br style="color: rgb(153, 153, 153);"> <span style="color: rgb(153, 153, 153);">be dealt with in accordance with copyright and privacy legislation.</span><br style="color: rgb(153, 153, 153);"> <span style="color: rgb(153, 153, 153);">Except where otherwise stated, views expressed are those of the individual sender</span>. </body> </html> |
From: Demian K. <dem...@vi...> - 2010-03-03 16:12:17
|
> I don't know the code well enough to know whether it is feasible to > decouple the template paths from the RecordDrivers (or even if it is > worth it). First of all, thanks for the testing and feedback -- I'm glad to hear you were able to work with the code with relative ease! It made sense in my head, but that's not always proof of accessibility to the rest of the world. As far as the issue of template paths goes, I figured there were three obvious approaches: 1.) Give each driver its own template directory, which would force redundant templates in many cases -- a headache to maintain over time. 2.) Make each driver explicitly specify template paths, which allows easy sharing of templates but requires extra code when all you want to do is override a template name. 3.) Make each driver have a search path for templates to allow inheritance and overriding -- the most flexible from the user perspective, but the most complicated from an initial implementation perspective, and a potential (minor) performance drain. As you noticed, I ended up implementing #2 -- #1 seemed too likely to cause problems (it's bad enough that we have to maintain three different themes, but unnecessarily adding multiple versions of certain content to each theme is exponentially worse) and #3 seemed like a lot of extra work for a marginal benefit. Fortunately, this is not difficult to change if necessary -- it's an implementation detail of the IndexRecord driver and its children, but changing it has no impact on the overall record driver interface design. One possible easy solution would be to replace all the hard-coded template paths with calls to this method: protected function getTemplatePath($template, $folder = null) { if (is_null($folder)) { $folder = $this->defaultTemplateFolder; } return "RecordDrivers/{$folder}/{$template}"; } This allows you to set a default template path but override it through the second $folder parameter. You could easily change all the template paths by extending a class and changing $this->defaultTemplateFolder, or you could make special-case exceptions through use of the $folder parameter. I'm really not sure if it's worth it, though -- while it simplifies theming in some ways, it adds another detail that needs to be understood before extending the class. Perhaps a bit of awkward method-extending is actually better than having a more engineered but less obvious solution. Or perhaps not. Let me know what you think -- nothing is set in stone yet. - Demian |
From: <Pet...@pa...> - 2010-03-04 02:13:00
|
Demian, Thanks for explaining your reasoning. I think it is always wise to be pragmatic about these kinds of things and your approach makes sense. I am happy to go with it as it stands. I might add a bit on the RecordDriver part of the wiki to explain how themes interact with RecordDrivers (I think a few clues there would have saved me a bit of time digging around in the code). Cheers, Peter -- Peter Neish Systems Officer Victorian Parliamentary Library Ph: 03 9651 8638 pet...@pa... Demian Katz <dem...@vi...> wrote on 04/03/2010 03:09:57 AM: > From: Demian Katz <dem...@vi...> > To: "Pet...@pa..." > <Pet...@pa...>, "vufind- > te...@li..." <vuf...@li...> > Date: 04/03/2010 03:12 AM > Subject: Re: [VuFind-Tech] Please test/comment -- record driver > refactoring complete > > > I don't know the code well enough to know whether it is feasible to > > decouple the template paths from the RecordDrivers (or even if it is > > worth it). > > First of all, thanks for the testing and feedback -- I'm glad to > hear you were able to work with the code with relative ease! It > made sense in my head, but that's not always proof of accessibility > to the rest of the world. > > As far as the issue of template paths goes, I figured there were > three obvious approaches: > > 1.) Give each driver its own template directory, which would force > redundant templates in many cases -- a headache to maintain over time. > 2.) Make each driver explicitly specify template paths, which allows > easy sharing of templates but requires extra code when all you want > to do is override a template name. > 3.) Make each driver have a search path for templates to allow > inheritance and overriding -- the most flexible from the user > perspective, but the most complicated from an initial implementation > perspective, and a potential (minor) performance drain. > > As you noticed, I ended up implementing #2 -- #1 seemed too likely > to cause problems (it's bad enough that we have to maintain three > different themes, but unnecessarily adding multiple versions of > certain content to each theme is exponentially worse) and #3 seemed > like a lot of extra work for a marginal benefit. > > Fortunately, this is not difficult to change if necessary -- it's an > implementation detail of the IndexRecord driver and its children, > but changing it has no impact on the overall record driver interface design. > > One possible easy solution would be to replace all the hard-coded > template paths with calls to this method: > > protected function getTemplatePath($template, $folder = null) > { > if (is_null($folder)) { > $folder = $this->defaultTemplateFolder; > } > return "RecordDrivers/{$folder}/{$template}"; > } > > This allows you to set a default template path but override it > through the second $folder parameter. You could easily change all > the template paths by extending a class and changing > $this->defaultTemplateFolder, or you could make special-case > exceptions through use of the $folder parameter. > > I'm really not sure if it's worth it, though -- while it simplifies > theming in some ways, it adds another detail that needs to be > understood before extending the class. Perhaps a bit of awkward > method-extending is actually better than having a more engineered > but less obvious solution. Or perhaps not. Let me know what you > think -- nothing is set in stone yet. > > - Demian > > ------------------------------------------------------------------------------ > Download Intel® Parallel Studio Eval > Try the new software tools for yourself. Speed compiling, find bugs > proactively, and fine-tune applications for parallel performance. > See why Intel Parallel Studio got high marks during beta. > http://p.sf.net/sfu/intel-sw-dev > _______________________________________________ > Vufind-tech mailing list > Vuf...@li... > https://lists.sourceforge.net/lists/listinfo/vufind-tech ///////************************************************************/////////////// Parliament of Victoria . Important Disclaimer Notice: The information contained in this email including any attachments, may be confidential and/or privileged. If you are not the intended recipient, please notify the sender and delete it from your system. Any unauthorised disclosure, copying or dissemination of all or part of this email, including any attachments, is not permitted. This email, including any attachments, should be dealt with in accordance with copyright and privacy legislation. Except where otherwise stated, views expressed are those of the individual sender. |
From: Demian K. <dem...@vi...> - 2010-03-04 13:25:18
|
> Thanks for explaining your reasoning. I think it is always wise to be > pragmatic about these kinds of things and your approach makes sense. I am > happy to go with it as it stands. I might add a bit on the RecordDriver > part of the wiki to explain how themes interact with RecordDrivers (I think > a few clues there would have saved me a bit of time digging around in the > code). Sounds good to me -- improvements to the documentation are always welcome! - Demian |