Ferreteria/archive/changes/1: Difference between revisions

From Woozle Writes Code
Jump to navigation Jump to search
(status update)
m (6 revisions imported: moving this project here)
 
(4 intermediate revisions by one other user not shown)
Line 1: Line 1:
<hide>
<hide>
[[affects piece::forms]]
[[affects piece::forms]]
[[subject::parameter change to clsForm_recs::Save()]]
[[subject::parameter change to clsForm_recs.Save()]]
</hide>
</hide>
'''Change to''': form-data.php:clsForm recs::Save() (see {{l/ferreteria/|pieces/forms|forms}})
==Existing situation==
==Existing situation==
Save() formerly expected two optional parameters, <code>$iNotes</code> and <code>array $arRedir=NULL</code>. At the time of the change, there were a small percentage of callers actually using the second parameter, and from what I can tell they were only using it to reset the path to the record's default -- i.e. eliminating any commands or options such as "edit".
Save() formerly expected two optional parameters, <code>$iNotes</code> and <code>array $arRedir=NULL</code>. At the time of the change, there were a small percentage of callers actually using the second parameter, and from what I can tell they were only using it to reset the path to the record's default -- i.e. eliminating any commands or options such as "edit".

Latest revision as of 16:42, 22 May 2022

<hide> affects piece::forms subject::parameter change to clsForm_recs.Save() </hide> Change to: form-data.php:clsForm recs::Save() (see forms)

Existing situation

Save() formerly expected two optional parameters, $iNotes and array $arRedir=NULL. At the time of the change, there were a small percentage of callers actually using the second parameter, and from what I can tell they were only using it to reset the path to the record's default -- i.e. eliminating any commands or options such as "edit".

Need for change

I wanted to call it with a string parameter instead, so that the form's post-save redirect could go to a completely different record's default URL (retrieved from $this->AdminURL()) -- specifically, I was implementing a "new order" form to be displayed within a "customer" page, and wanted to redirect back to the customer page rather than that of the newly-created order record.

Options considered

I first tried adding the string as a third parameter, where the caller could provide either the array or the string (or neither). However, there was already one descendant implementation of Save(), so PHP balked because the parameters no longer matched -- and I noticed that the descendant wasn't even using the parameter, so it seemed like time to rethink the parameter list.

I next considered replacing the array parameter with the string parameter, and throwing an exception if an array was detected in order to remind myself of the change so I could see if the array was actually necessary for those few callers using it, or if there was some better way to accomplish the same thing -- but this still left the inelegance of a required parameter that was never used in the descendant class.

Changes made

I finally decided that this complication made it clear that redirection should be the caller's job -- with $this->AdminRedirect() being generally available (it may not have been when this function was originally written) -- thus eliminating the need to pass any redirection path information at all.

  • removed $arRedir from the parameter list
  • removed all the code associated with it
  • saved $out as $this->htMsg, so caller can display it if there's an opportunity
  • function now returns the value of $ok instead of $out
  • went through all known callers, and made sure they do their own redirection -- typical code:

<php> $oForm = $this->PageForm(); if ($oForm->Save()) { $this->AdminRedirect(); } else { echo $oForm->htMsg; } </php> It's possible that this was unnecessary in some places, but I figure I'll tidy those up as I get to them.