I am attempting to migrate some PHP code from 4.4 to 5.2. I am new to PHP and OOP, but I seem to be doing okay so far. However, there is one particular section of code I am having difficulty with, and web searches have not helped me understand what I need to do here so far. I have copied what I believe are the relevant sections of the code in question below and marked the problem line with //PROBLEM LINE IS HERE. I would be very grateful for any assistance.
class Event {
function Event($clinician,$now) {
global $username;
$this->InDB = '';
$this->NoteID = '';
$this->EntryFor = $clinician;
$this->TimeStamp = $now;
$this->EntryBy = $username;
$this->AtStep = '2';
$this->Location = '';
$this->ChiNumStart = '';
$this->GroupId = '0';
$this->Date = '';
$this->StartTime = '';
$this->EndTime = '';
$this->TravelTime = '0';
$this->PreparationTime = '0';
$this->ParticipationTime = '';
$this->ReviewTime = '0';
$this->ServiceList = array();
$this->ThisClinician = new Clinician();
$this->OtherCliniciansList = array();
$this->MembersList = array();
$this->NonMembersList = array();
$this->CoSignature = '';
$this->NoteType = '';
$this->ProgressNote = new ProgressNote();
$this->IRAP = new IRAP();
$this->PCITNote = new PCIT();
$this->NoteWritingTime = '';
$this->ErrorList = array();
$this->Debug = 0;
}
function loadFromDB($event_id,$clinician) {
require_once("../includes/connect.php");
// PROBLEM LINE IS HERE
$this = new Event($clinician,strtotime('now'));
$event_table = mysql_fetch_object(mysql_query("select * from events where id like '$event_id'"));
$this->InDB = $event_table->id;
$this->TimeStamp = $event_table->entry_date;
$this->Date = $event_table->date;
$this->StartTime = $event_table->start_time;
$this->EndTime = $event_table->end_time;
$this->ChiNumStart = $event_table->chi_num_start;
$this->GroupId = $event_table->group_id;
$this->Location = $event_table->location;
$this->AtStep = $event_table->at_step;
$this->EntryFor = $event_table->entry_for;
$this->CoSignature = $event_table->co_signature;
$this->NoteWritingTime = $event_table->note_writing_time;
$this->NoteType = $event_table->note_type;
$this->NoteID = $event_table->note_id;
$event_services_table = mysql_query("select * from event_services where event_id like '$event_id' and active like '1'");
while($service = mysql_fetch_object($event_services_table)) {
$new_service = new Service($service->service_id,$service->service_title);
$new_service->OverrideCode = $service->override_code;
$this->ServiceList[] = $new_service;
}
$clinician_res = mysql_query("select * from event_clinicians where event_id like '$event_id' and active like '1'");
while($clinician_table = mysql_fetch_object($clinician_res)) {
if(strcmp($clinician_table->clinician_name,$this->EntryFor)) {
$add_clinician = new Clinician();
$add_clinician->Name = $clinician_table->clinician_name;
$add_clinician->ID = $clinician_table->clinician_id;
$add_clinician->Present = $clinician_table->present;
$add_clinician->PreparationTime = $clinician_table->prep_time;
$add_clinician->ParticipationTime = $clinician_table->participation_time;
$add_clinician->ReviewTime = $clinician_table->review_time;
$add_clinician->TravelTime = $clinician_table->travel_time;
$add_clinician->MileageStart = $clinician_table->mileage_start;
$add_clinician->MileageEnd = $clinician_table->mileage_end;
$add_clinician->Authorized = $clinician_table->authorized;
$add_clinician->Role = $clinician_table->role;
$this->OtherCliniciansList[] = $add_clinician;
} else {
$this->PreparationTime = $clinician_table->prep_time;
$this->ParticipationTime = $clinician_table->participation_time;
$this->ReviewTime = $clinician_table->review_time;
$this->TravelTime = $clinician_table->travel_time;
$this->MileageStart = $clinician_table->mileage_start;
$this->MileageEnd = $clinician_table->mileage_end;
}
}
}
}
Is $this = new Event($clinician,strtotime('now')); just calling the class constructor function and can be replaced with $this->Event($clinician,strtotime('now'));? Or does it create a new blank event whenever the function is called in which case maybe I should remove the line and then add a $variablename = new Event ($clinician,strtotime('now')); line before any $variablename->loadFromDB($event_id,$clinician) lines in order to create the new instance before calling the function? Or am I just entirely misunderstanding what the effect of this line is?"$this" refers to the current object instance of the class in which it is used. You cannot therefore overwrite it with another object, which is what the "new" keyword creates. As currently written, the loadFromDb() method is trying to create an Event object within an Event object, which probably is not what you want to do."$this" refers to the current object instance of the class in which it is used. You cannot therefore overwrite it with another object, which is what the "new" keyword creates. As currently written, the loadFromDb() method is trying to create an Event object within an Event object, which probably is not what you want to do.
That pretty much sums up my current understanding of the situation... the problem is, the original programmer presumably put it there for a reason, and it is apparently doing -something- under PHP 4.4. Without knowing WHY it is there, I don't know what to do to make it compatible with 5.2.
The concept of re-assigning $this makes no sense me... if the original programmer wanted to create a new event object inside the current event object then I would expect to see $this->$variablename = new Event() and not $this = new event()... and that wouldn't be re-assigning $this itself.
The code seems to imply that the programmer is replacing the current event object with a new event object... but what happens to the "state" of the object if that is the case? I would assume the resulting object's properties would be that of a new Event() object... which would result in the following line of code performing a select querry on the unset variable $event_id... unless the variables inside the function are somehow passed on to the "new" object. In that case wouldn't it have been simpler to just create a new object and then invoke the loadFromDB method on new object instead to trying to re-assign $this? Since this doesn't make any sense to me, I've been wondering if perhaps the effect of the re-assignment was simply to call the constructor function of the current event object... in which case why did they not simply write the line as a simple function call to the constructor instead of writing it as a re-assginment?Maybe change the offending line to...
$this->Event($clinician,strtotime('now'));
...making it call the constructor again?That was my gut instinct too.... I'm going to try it, though it might take quite a while before I can be sure it is working as intended. Thus are the joys of inheriting unique, undocumented, and uncommented code designed to provide provide solutions to unique, undocumented, and uncommented problems
.Personally I'd make that a static method that returned a new Event object.
$event = Event::loadFromDB($event_id, $clinician);
The other alternative would be to instantiate an Event object, and then call that method on it but instead of trying to create a new object and calling it $this, just update $this's properties.
$event = new Event($clinician, time()); // strtotime('now')?!
$event->loadFromDB($event_id); // the $event already has its $clinician.
Either way would need changes, but since it doesn't run now, that's kind of inevitableThank you both for your help.
I initially replaced the offending line of code with a simple call to the constructor function. In testing this appeared to work... but wasn't readily apparent if ALL the object properties were being replaced by the database querry or if some were carrying over from the existing object's state.
I searched the code and found only two locations where that method is called, so I deleted the offending line entirely, removed the method's second pramater, and replaced the two method calls with:
$event = new Event( ''. '' );
$event->loadFromDB( $event_id );
This also seems to work in testing and I think it makes what is happening clearer. Now that I know this works, I like the idea of making it a static method that returns a new event object and calling it like: $event = Event::loadFromDB($event_id); but that will require a little refactoring of the class and I am not ready to tackle that project yet. However I have added it to my to do list for that section of the code.
Thanks again for your help.. it looks like I was able to fix the rest of the upgrade issues and have the application running on current versions of mySQL, PHP, and Apache. Next up.. fixing the stupid reliance on register_globals for form submissions and session_register for session handling.
class Event {
function Event($clinician,$now) {
global $username;
$this->InDB = '';
$this->NoteID = '';
$this->EntryFor = $clinician;
$this->TimeStamp = $now;
$this->EntryBy = $username;
$this->AtStep = '2';
$this->Location = '';
$this->ChiNumStart = '';
$this->GroupId = '0';
$this->Date = '';
$this->StartTime = '';
$this->EndTime = '';
$this->TravelTime = '0';
$this->PreparationTime = '0';
$this->ParticipationTime = '';
$this->ReviewTime = '0';
$this->ServiceList = array();
$this->ThisClinician = new Clinician();
$this->OtherCliniciansList = array();
$this->MembersList = array();
$this->NonMembersList = array();
$this->CoSignature = '';
$this->NoteType = '';
$this->ProgressNote = new ProgressNote();
$this->IRAP = new IRAP();
$this->PCITNote = new PCIT();
$this->NoteWritingTime = '';
$this->ErrorList = array();
$this->Debug = 0;
}
function loadFromDB($event_id,$clinician) {
require_once("../includes/connect.php");
// PROBLEM LINE IS HERE
$this = new Event($clinician,strtotime('now'));
$event_table = mysql_fetch_object(mysql_query("select * from events where id like '$event_id'"));
$this->InDB = $event_table->id;
$this->TimeStamp = $event_table->entry_date;
$this->Date = $event_table->date;
$this->StartTime = $event_table->start_time;
$this->EndTime = $event_table->end_time;
$this->ChiNumStart = $event_table->chi_num_start;
$this->GroupId = $event_table->group_id;
$this->Location = $event_table->location;
$this->AtStep = $event_table->at_step;
$this->EntryFor = $event_table->entry_for;
$this->CoSignature = $event_table->co_signature;
$this->NoteWritingTime = $event_table->note_writing_time;
$this->NoteType = $event_table->note_type;
$this->NoteID = $event_table->note_id;
$event_services_table = mysql_query("select * from event_services where event_id like '$event_id' and active like '1'");
while($service = mysql_fetch_object($event_services_table)) {
$new_service = new Service($service->service_id,$service->service_title);
$new_service->OverrideCode = $service->override_code;
$this->ServiceList[] = $new_service;
}
$clinician_res = mysql_query("select * from event_clinicians where event_id like '$event_id' and active like '1'");
while($clinician_table = mysql_fetch_object($clinician_res)) {
if(strcmp($clinician_table->clinician_name,$this->EntryFor)) {
$add_clinician = new Clinician();
$add_clinician->Name = $clinician_table->clinician_name;
$add_clinician->ID = $clinician_table->clinician_id;
$add_clinician->Present = $clinician_table->present;
$add_clinician->PreparationTime = $clinician_table->prep_time;
$add_clinician->ParticipationTime = $clinician_table->participation_time;
$add_clinician->ReviewTime = $clinician_table->review_time;
$add_clinician->TravelTime = $clinician_table->travel_time;
$add_clinician->MileageStart = $clinician_table->mileage_start;
$add_clinician->MileageEnd = $clinician_table->mileage_end;
$add_clinician->Authorized = $clinician_table->authorized;
$add_clinician->Role = $clinician_table->role;
$this->OtherCliniciansList[] = $add_clinician;
} else {
$this->PreparationTime = $clinician_table->prep_time;
$this->ParticipationTime = $clinician_table->participation_time;
$this->ReviewTime = $clinician_table->review_time;
$this->TravelTime = $clinician_table->travel_time;
$this->MileageStart = $clinician_table->mileage_start;
$this->MileageEnd = $clinician_table->mileage_end;
}
}
}
}
Is $this = new Event($clinician,strtotime('now')); just calling the class constructor function and can be replaced with $this->Event($clinician,strtotime('now'));? Or does it create a new blank event whenever the function is called in which case maybe I should remove the line and then add a $variablename = new Event ($clinician,strtotime('now')); line before any $variablename->loadFromDB($event_id,$clinician) lines in order to create the new instance before calling the function? Or am I just entirely misunderstanding what the effect of this line is?"$this" refers to the current object instance of the class in which it is used. You cannot therefore overwrite it with another object, which is what the "new" keyword creates. As currently written, the loadFromDb() method is trying to create an Event object within an Event object, which probably is not what you want to do."$this" refers to the current object instance of the class in which it is used. You cannot therefore overwrite it with another object, which is what the "new" keyword creates. As currently written, the loadFromDb() method is trying to create an Event object within an Event object, which probably is not what you want to do.
That pretty much sums up my current understanding of the situation... the problem is, the original programmer presumably put it there for a reason, and it is apparently doing -something- under PHP 4.4. Without knowing WHY it is there, I don't know what to do to make it compatible with 5.2.
The concept of re-assigning $this makes no sense me... if the original programmer wanted to create a new event object inside the current event object then I would expect to see $this->$variablename = new Event() and not $this = new event()... and that wouldn't be re-assigning $this itself.
The code seems to imply that the programmer is replacing the current event object with a new event object... but what happens to the "state" of the object if that is the case? I would assume the resulting object's properties would be that of a new Event() object... which would result in the following line of code performing a select querry on the unset variable $event_id... unless the variables inside the function are somehow passed on to the "new" object. In that case wouldn't it have been simpler to just create a new object and then invoke the loadFromDB method on new object instead to trying to re-assign $this? Since this doesn't make any sense to me, I've been wondering if perhaps the effect of the re-assignment was simply to call the constructor function of the current event object... in which case why did they not simply write the line as a simple function call to the constructor instead of writing it as a re-assginment?Maybe change the offending line to...
$this->Event($clinician,strtotime('now'));
...making it call the constructor again?That was my gut instinct too.... I'm going to try it, though it might take quite a while before I can be sure it is working as intended. Thus are the joys of inheriting unique, undocumented, and uncommented code designed to provide provide solutions to unique, undocumented, and uncommented problems
.Personally I'd make that a static method that returned a new Event object.$event = Event::loadFromDB($event_id, $clinician);
The other alternative would be to instantiate an Event object, and then call that method on it but instead of trying to create a new object and calling it $this, just update $this's properties.
$event = new Event($clinician, time()); // strtotime('now')?!
$event->loadFromDB($event_id); // the $event already has its $clinician.
Either way would need changes, but since it doesn't run now, that's kind of inevitable
Thank you both for your help.I initially replaced the offending line of code with a simple call to the constructor function. In testing this appeared to work... but wasn't readily apparent if ALL the object properties were being replaced by the database querry or if some were carrying over from the existing object's state.
I searched the code and found only two locations where that method is called, so I deleted the offending line entirely, removed the method's second pramater, and replaced the two method calls with:
$event = new Event( ''. '' );
$event->loadFromDB( $event_id );
This also seems to work in testing and I think it makes what is happening clearer. Now that I know this works, I like the idea of making it a static method that returns a new event object and calling it like: $event = Event::loadFromDB($event_id); but that will require a little refactoring of the class and I am not ready to tackle that project yet. However I have added it to my to do list for that section of the code.
Thanks again for your help.. it looks like I was able to fix the rest of the upgrade issues and have the application running on current versions of mySQL, PHP, and Apache. Next up.. fixing the stupid reliance on register_globals for form submissions and session_register for session handling.