Re: [VuFind-Tech] Fines Screen changes

SourceForge Headquarters 225 Broadway Suite 1600 San Diego, CA 92101 +1 (858) 454-5900

Thanks -- it's working for me now.  One last minor point, though -- function_exists actually takes a string for a parameter.  What's actually happening in the latest patch is that PHP thinks you're passing in a constant, and when it finds that the constant isn't defined, it defaults it to the constant name as a string... so it works, but in a roundabout way.  This "constant defaulting to string" thing is one of the great traps PHP sets to help in the creation of really confusing bugs.  In the rare but not impossible case that one of these constants suddenly becomes defined somewhere down the road, crazy things can start happening!

The driver champion idea is definitely a good one -- this will be helpful not just for future development, but also for confirming that all the current drivers behave consistently once we've documented what we think they're actually supposed to be doing.  I'm happy to take on Voyager duties.  Do you think we should put in a Wiki page to track this?

- Demian

From: Greg Pendlebury [mailto:Gre...@us...]
Sent: Wednesday, August 19, 2009 9:43 PM
To: Demian Katz; vuf...@li...
Subject: RE: Fines Screen changes

1) Good point. I've called it 'finesIndexEngine' to match what we call the index in the config file.

2) I've reverted the change and I'll modify our Virtua driver to return cents and the Demo driver as well. On this and other matters, perhaps we should start to look for individuals/libraries willing to 'champion' their driver, so we have a point of contact for changes like this (I'm happy to code, but I can't test). This integer vs. floating point issue is pretty trivial, but the other work being discussed on the list will required some significant re-coding soon I imagine.

3) Ok, it tests for the function's existance first, then uses the new function as a fallback. I'm a lot happier with this too since I trust windows to make locale info available a lot more consistently (It at least ships with US centric defaults instead of blank values). I've morphed my layout slightly too. I can't promise I'll remember every time, but I'll try :)

I've reverted r1302 and patch r1310 is here:

http://vufind.svn.sourceforge.net/viewvc/vufind/branches/usq/web/services/MyResearch/Fines.php?view=patch&r1=1310&r2=1309&pathrev=1310

Greg Pendlebury
Electronic Services Officer (Systems Team)
Division of Academic Information Services
University of Southern Queensland
Phone: +61 7 4631 1501
Fax: +61 7 4631 1841

________________________________
From: Demian Katz [mailto:dem...@vi...]
Sent: Thursday, 20 August 2009 12:03 AM
To: Greg Pendlebury; vuf...@li...
Subject: RE: Fines Screen changes
1.) Pardon if I'm just nitpicking, but a few thoughts on your $solr_index variable name -- a) it should probably mention "fines" in some fashion to further reduce the chances of future name clashes; b) I get the impression that at least some of the code tries to be fairly abstract about our underlying index mechanism, and I think this is a good goal, so I don't think we want to include "solr" in the name; c) most of our variables use camelCase, and it's probably best to stick with that for consistency.  Given those comments, perhaps finesDb or finesDbRef would be a better name.

2.) This is an area where better documentation of the driver interface would be extremely helpful.  Looks like Voyager's database stores fine amounts as pennies rather than dollars, and VuFind has been designed around this assumption.  I agree that it might make more sense to return the floating point number, but if you change the fines module without also changing the Voyager driver, things will definitely not work as expected!  I'm not sure what any of the other ILS drivers are currently doing in this regard but I would guess that they're consistent with Voyager or else we would be getting bug reports about crazy fines.  Documenting the driver interface is something I'd eventually like to tackle, but it's a large and labor-intensive project, so it's probably going to be a while before I get to it.  In the meantime, I think we have to proceed with caution -- unless you want to analyze and change all of the drivers, it might be better to stick with the pennies assumption for the moment in the interest of maintaining stability.

3.) safe_money_format() isn't working for me on my Ubuntu test box.  My localeconv() values are listed below, and they cause crazy results (not sure where those 127's are coming from -- the documentation doesn't address them).  money_format works just fine for me.  Perhaps we should set up safe_money_format as a failback only?  Also, if we do end up including the function, we should probably reformat it to reflect VuFind's style conventions.

array(18) {
  ["decimal_point"]=>
  string(1) "."
  ["thousands_sep"]=>
  string(0) ""
  ["int_curr_symbol"]=>
  string(0) ""
  ["currency_symbol"]=>
  string(0) ""
  ["mon_decimal_point"]=>
  string(0) ""
  ["mon_thousands_sep"]=>
  string(0) ""
  ["positive_sign"]=>
  string(0) ""
  ["negative_sign"]=>
  string(0) ""
  ["int_frac_digits"]=>
  int(127)
  ["frac_digits"]=>
  int(127)
  ["p_cs_precedes"]=>
  int(127)
  ["p_sep_by_space"]=>
  int(127)
  ["n_cs_precedes"]=>
  int(127)
  ["n_sep_by_space"]=>
  int(127)
  ["p_sign_posn"]=>
  int(127)
  ["n_sign_posn"]=>
  int(127)
  ["grouping"]=>
  array(0) {
  }
  ["mon_grouping"]=>
  array(0) {
  }
}

- Demian

From: Greg Pendlebury [mailto:Gre...@us...]
Sent: Tuesday, August 18, 2009 8:03 PM
To: vuf...@li...
Subject: [VuFind-Tech] Fines Screen changes

I've attached a patch to Fines.php in the MyResearch area that does a few tweaks:

1) Changes the global '$db' to '$solr_index' because '$db' was already in use and would throw warnings. (minor bug).

2) Removed this line:
'$number = substr($record[$fieldName], 0, -2) . '.' . substr($record[$fieldName], -2);'
I'm not really sure where it came from but (if I'm reading it right) it meant that the fines screen was expecting the ILMS driver to return values of '500' for $5.00.
I'd think we're better off having the ILMS driver return '5.00', but please let me know if anyone wildly disagrees.

3) (and the biggest) The money_format() function is not available under a windows php install, so I've shamelessly stolen one of the examples from php.net and added safe_money_format() to the file to replace it.

Part of that change involves how to get currency information. The present method in index.php doesn't work on either of my test environments, but adding this: 'setlocale(LC_ALL, '');' is supposed to call out to the OS environment variables to get the info. It's working for me now on both Windows and Solaris.

This pre-supposes that you will be running your install on a local server. I'd prefer the config file method (So you can host someone else's locale on your server) but can't seem to get it working with anything like:
'en_AU', 'en_AU.utf8' or even the 'en_US' values.

Any feedback would be appreciated. It's currently commited in the USQ branch (r1302), and I'll move it to trunk after tweaking based on feedback (if there is any).

Ta,

Greg Pendlebury
Electronic Services Officer (Systems Team)
Division of Academic Information Services
University of Southern Queensland
Phone: +61 7 4631 1501
Fax: +61 7 4631 1841

________________________________
This email (including any attached files) is confidential and is for the intended recipient(s) only. If you received this email by mistake, please, as a courtesy, tell the sender, then delete this email.

The views and opinions are the originator's and do not necessarily reflect those of the University of Southern Queensland. Although all reasonable precautions were taken to ensure that this email contained no viruses at the time it was sent we accept no liability for any losses arising from its receipt.

The University of Southern Queensland is a registered provider of education with the Australian Government (CRICOS Institution Code No's. QLD 00244B / NSW 02225M)
________________________________
This email (including any attached files) is confidential and is for the intended recipient(s) only. If you received this email by mistake, please, as a courtesy, tell the sender, then delete this email.

The views and opinions are the originator's and do not necessarily reflect those of the University of Southern Queensland. Although all reasonable precautions were taken to ensure that this email contained no viruses at the time it was sent we accept no liability for any losses arising from its receipt.

The University of Southern Queensland is a registered provider of education with the Australian Government (CRICOS Institution Code No's. QLD 00244B / NSW 02225M)