|
PHP - Refactoring Question (function Side-effects, Class Globals)
Imagine 6 PHP classes (one each for a product line), that have very similar coding structures, that go like this:
//function that computes stuff inside each of 6 files:
//they vary slightly from file to file but essentially it is this:
function computeFunction
{
$this->x = new X();
$this->x->calcD();
if ($this->x->dOk)
{
$this->x->calcE();
$this->x->calcN();
}
//more complicated logic that is essentially like above
//and by the way!
print $this->x->someVarThatIsUsedLater;
}
Then there is a single class like so :
class X
{
function calcD()
{
//compute some condition
if (<computed condition is met>) $this->dOk = true;
else $this->dOk = false;
//and by the way
$this->someVarThatIsUsedLater = 4;
}
}
Just to bring your attention to it, none of these functions return any result or value, but they nevertheless operate on variables of key interest via side-effects. That is, they modify variables that essentially act like globals, and then use those variables later ($this->dOk and $this->someVarThatIsUsedLater are one more prominent examples).
I need to untangle this mess. And make it clean and clear again, and make sense. How do I best proceed?
I have been wrestling with some ideas... like $this->dOk, can within reason be turned into a return variable of calcD() function, and then be tested against like if ($this->x->calcD()) and I think it will be reasonable enough. But then there are other functions that don't return anything and just act on variables via side-effects anyway so $this->dOk is one of the lesser troubles...
Other than that, what I am thinking of doing is getting rid of these mini-functions (calcE(), calcN(), etc.), removing them as a funciton, and putting their body directly into the code, as a first step to refactor. Many of the computations done inside are just a few lines of code anyway, and the functions kind of hide a lot of side-effects that happen, instead of actually encapsulating the behavior. So while it may be counter-intuitive to dismantle the functions that appear to be doing something that normally can be encapsulated (computing key variables E, N, etc), I think dismantling them will actually clean things up as far as collecting all the side-effects inside a single parent function thereby making them more visible.
Caveat: while doing so I will end up with 6 copies of untangled dismantled functions, because dismantling class X and putting its content into each of the 6 product line classes will have that effect.
But my hope is that from that point I will see more clearly to start identifying places where I can start to truly encapsulating the behavior via various structures, instead of masking it.
Problems / Questions:
I would like to but I am not entirely sure that I can skip that step of dismantling functions & the 6x multiplying effect. It's probably the same like skipping steps in solving polynomial equations. Some can do it and some need to list each step of their work.
And I am not entirely sure what structures I can replace it with in the end after I dismantle the functions.
It also looks like a lot of work. Is there a better way?
P.S. I already put tests on computeFunction() for each product line so I can be less paranoid about hacking stuff up.
Edited by dennis-fedco, 19 January 2015 - 03:06 PM. Similar TutorialsI have a class that adds a special PDF page to an existing PDF. The page refers to a specific product and there are products A, B, C, D. Products B, C, D are similar and with a bit of if/then/else magic I can basically reuse the same function for all 3, but product A is different enough to have its own function.
My class looks like this:
class AddPdfPage
{
function addA(&$pdf, $vars) {}
function addB(&$pdf, $vars) {}
function addC(&$pdf, $vars) {}
function addD(&$pdf, $vars) {}
}
//The way I call it from within the code right now is like this:
AddPdfPage::addC($pdf, $vars);
AddPdfPage::addB($pdf, $vars);
//the variable below contains the product line -- A, B, C, or D
//that's the one I said I could use to merge functions B/C/D into one if I wanted to
//I could even include product A if I wanted to even though the code there is different.
$vars['productLine'];
//The code inside addX() functions is something like:
function addX($pdf, $vars)
{
$pdf->addPage($vars);
$pdf->addDescription(".."); //that is different based on each specific product line
}
Assuming I want to add a page to PDF for a paricular product X, how do I write or structure my code? I am not exactly sure.
Do I set it up somehow to use polymorphism? Do I do something else?
I assume I don't really want to merge products B/C/D into the same function, even if I could, since they physically represent different product lines, so I might as well keep them as such -- separate. But I may want to use some features of the language to clean up and better up my code. The "how", and what techniques do I use, and what my final code may look like is what my post is about.
I am thinking of doing something like:
class AddPdfPage()
{
private $pdf;
private $vars;
private $productLine;
function __construct($pdf, $vars)
{
$this->pdf = $pdf;
$this->vars = $vars;
$this->productLine = $vars['productLine'];
}
public function addPage()
{
switch($this->productLine)
{
case 'A': ...break;
case 'B': ...break;
case 'C': ...break;
case 'D': ...break;
}
}
}
Use that as a jump start and I guess I could use polymorphism by creating separate classes for each product line and extending some base class, if switch statement is not good.. (http://c2.com/cgi/wi...StatementsSmell)
but I feel like I will be polluting my file system with too many classes and not sure if that's exactly a benefit. So just thinking about how to do this.
Edited by dennis-fedco, 15 October 2014 - 10:56 AM. I'm learning functions and I'm working on a rating script. This is a small test script that works, you can try it out yourself: <?php // Rating System function while_test (){ $a = 1; $b = 4; $t_id = 1; global $likes; global $dislikes; global $con_id; while ($a++ <= $b){ echo "<center>"; echo "<table><tr><td>Table: </td></tr>"; echo "<tr><td>This is a table test </td></tr>"; echo "<tr><td><form action='' method='post'>"; echo "<button type='submit' name='likes' value='Y'>likes</button>"; echo "<button type='submit' name='dislikes' value='N'>dislikes</button>"; echo "<input type='hidden' name='hidden_id' value='" . $t_id . "' /></form></td></tr></table>"; echo "</center><br /><br />"; $t_id++; $likes = $_POST['likes']; $dislikes = $_POST['dislikes']; $con_id = $_POST['hidden_id']; } } while_test(); if ($likes) { echo "likes it: " . $likes . " con_id: " . $con_id; } elseif ($dislikes) { echo "dislikes it: " . $dislikes . " con_id: " . $con_id; } ?> I've gotten recommended before not use globals, because the projects would become unmanageable, and I'm wondering how would I be able to avoid using globals in this example? I'm able to in-ject variables through the parenthesis, but I'm not able to out-ject variables, if that makes sense. (?) At least it doesn't work for me. How would I use those three variables $likes, $dislikes and $con_id outside the function without setting them as globals, what would be good practice? I just want to know how to create a global variable from within a function. The reason I want to do this is I'm making mysql queries that may or may not have data in a function, if the data does indeed exist, I want to declare variables at that point. My queries are grouped logically by year/month, and as a result I'm going to be appending data to existing variables if they exist so it makes more sense to just append it to what would be the global variable anyways instead of just passing large strings out of the function to just be appended anyways -- plus it prevents me from creating a bunch of pointless variables. I intend additionally take the variables created inside the function and then store them in an encompassing global array (pre-defined outside of function) and at the end of the script do a foreach through it so I can iterate through the variables to grab my data knowing that there won't be pointless crap in there. I'm trying to create global variables by using variable variables ($$whichever). The code giving me issues inside the function is just like this: Code: [Select] function SeekAndAppend ($vars, $being, $passed) { global $$whatever; // Trying to define it here global $array; // Calling predefined $array $array[] = "$$whatever"; // Passing just created global to the array for iteration later } When I iterate through the array the expected name of what would be the global variable name is there, but the global variable itself does not exist -- just at the function level. If someone has a recommendation on a better way to do it, I'll listen, but please don't turn it into a lecture. EDIT: Grammar & Clarity I need to call usort from a class function, and I'm puzzled about how to define the comparison function. I've tried to define the comparison function in the same class, but I can't get usort to call it. I found one hint that it will work if I make the comparison function static, but I tried that, and it didn't work for me. If I define the comparison function outside the class, it won't have access to object properties that it needs to operate. The only solution I can think of is to define the comparison function outside the class and put the object properties it needs in globals. Is there a cleaner way to do this? Hi all. I have a video upload page utilizing ffmpeg to convert to .flv format via ajax with progress bar. Currently I am having an issue where the user uploads the video however the same upload script also converts to .flv format (exec("ffmpeg -i etc..")) after the file is uploaded. This is working fine however if the user selects to move away from the page after the video is uploaded it is causing the users browser to lockup (and sometimes crash). How can I make the exec() command to convert the video format strictly run server side and allow the user to move away from the page even though the conversion is still processing? Thanks for any advise on how to handle this issue. Hi all I have some code I'd like to re-factor Code: [Select] <?php $question_1 = $form->getObject()->getQuestion1(); $question_2 = $form->getObject()->getQuestion2(); ?> <div class="admin"> <div> <?php echo $form['question_1']->renderLabel(); echo $question_1; ?> </div> </div> <div class="adminl"> <div> <?php echo $form['question_2']->renderLabel(); $q2 = str_replace("-", '<br />',$question_2); echo $q2; ?> </div> </div> I'd preferably like some kind of for loop, that will just increment the _1, _2 values. Can anyone offer some help? Thanks Hi I have a method, which is pretty large. It has a lot of 'duplicate' code and I'd like to refactor it: protected function doSave($con = null) { $choices_1 = $this->getValue('question_1'); $choices_2 = $this->getValue('question_2'); $choices_3 = $this->getValue('question_3'); $choices_4 = $this->getValue('question_4'); $serialized_ = ""; $first = true; foreach($choices_1 as $choice_1) { if(!$first) { $serialized_1 .= ","; } $first = false; $serialized_1 .= $choice_1; } foreach($choices_2 as $choice_2) { if(!$first) { $serialized_2 .= ","; } $first = false; $serialized_2 .= $choice_2; } foreach($choices_3 as $choice_3) { if(!$first) { $serialized_3 .= ","; } $first = false; $serialized_3 .= $choice_3; } foreach($choices_4 as $choice_4) { if(!$first) { $serialized_4 .= ","; } $first = true; $serialized_4 .= $choice_4; } $this->setValue('question_1', $serialized_1); $this->setValue('question_2', $serialized_2); $this->setValue('question_3', $serialized_3); $this->setValue('question_4', $serialized_4); return parent::doSave($con); } You can see most of the arrays have a '_1', '_2' after them, the same goes for the $serialized variables. What I'd like is to maybe have some kind of loop, that will reduce my code down, and rather than me typing all the code above, possibly increment the '_1', '_2' part upto the 5 iterations. Can anyone provide some help/code for me to carry this out? Regards Hi I have the following code: $image_1 = $value->getElementsByTagName("Image1"); $image1 = $image_1->item(0)->nodeValue; $image_2 = $value->getElementsByTagName("Image2"); $image2 = $image_2->item(0)->nodeValue; $image_3 = $value->getElementsByTagName("Image3"); $image3 = $image_3->item(0)->nodeValue; $filename_1 = basename($image1); $ch = curl_init ($image1); curl_setopt($ch, CURLOPT_HEADER, 0); curl_setopt($ch, CURLOPT_RETURNTRANSFER, 1); curl_setopt($ch, CURLOPT_BINARYTRANSFER,1); $rawdata=curl_exec ($ch); curl_close ($ch); $fp = fopen(Mage::getBaseDir('media') . DS . 'import/'.$filename_1,'w'); fwrite($fp, $rawdata); fclose($fp); $filename_2 = basename($image2); $ch = curl_init ($image2); curl_setopt($ch, CURLOPT_HEADER, 0); curl_setopt($ch, CURLOPT_RETURNTRANSFER, 1); curl_setopt($ch, CURLOPT_BINARYTRANSFER,1); $rawdata=curl_exec ($ch); curl_close ($ch); $fp = fopen(Mage::getBaseDir('media') . DS . 'import/'.$filename_2,'w'); fwrite($fp, $rawdata); fclose($fp); $filename_3 = basename($image3); $ch = curl_init ($image3); curl_setopt($ch, CURLOPT_HEADER, 0); curl_setopt($ch, CURLOPT_RETURNTRANSFER, 1); curl_setopt($ch, CURLOPT_BINARYTRANSFER,1); $rawdata=curl_exec ($ch); curl_close ($ch); $fp = fopen(Mage::getBaseDir('media') . DS . 'import/'.$filename_3,'w'); fwrite($fp, $rawdata); fclose($fp); Would someone help me to refactor this, as I can seem needing $image_4, $image_5 at a later date. Thank you I was working on this project earlier on in the year, I have not posted here much. I would like to get it over with and start something else. I have other files which look like the excerpt of code from a file below. I personally think that my code lacks structure and could be organized in a better fashion. I lost most of my progress and am having to backtrack and restore code. Is there a way to re-write this code and make it more readable? Please and thank you!
<?php
include('header.php');
require_once('dbcon/dbcon.php');
//include('functions.php');
if ($_SERVER['REQUEST_METHOD'] == 'POST') {
// sanitize values before entering them into db, no bad seeds.
$username = mysqli_real_escape_string($conn, $_POST['username']);
$password = mysqli_real_escape_string($conn, $_POST['password']);
$bio = mysqli_real_escape_string($conn, $_POST['bio']);
$hashed_password = mysqli_real_escape_string($conn, password_hash($password, PASSWORD_DEFAULT));
$email = mysqli_real_escape_string($conn, $_POST['email_address']);
$confirmation_status = 0;
/* function sanitizeValues($x, string $postString) {
$x = mysqli_real_escape_string($conn, $_POST[$postString]);
}*/
$username_query = "SELECT * from profiles001 WHERE username='$username'";
$result = mysqli_query($conn, $username_query);
// if username exists do not continue...
if (mysqli_num_rows($result) > 0) {
header('Location: /soapbox/signup.php');
// let user know that username is taken...
} else {
// file upload stuff...
$file = $_FILES['file'];
$fileName = $_FILES['file']['name'];
$fileTmpName = $_FILES['file']['tmp_name'];
$fileSize = $_FILES['file']['size'];
$fileError = $_FILES['file']['error'];
$fileType = $_FILES['file']['type'];
$fileExt = explode('.', $fileName);
$fileActualExt = strtolower(end($fileExt));
$allowed = array('jpg', 'jpeg', 'png');
// avatar file constraints checks...
if (in_array($fileActualExt, $allowed)) {
if ($fileError === 0) {
if ($fileSize < 1000000) {
$fileNameNew = uniqid($_SESSION['username'], true) . "." . $fileActualExt;
$fileDestination = 'uploads/' . $fileNameNew;
move_uploaded_file($fileTmpName, $fileDestination);
} else {
echo "Your file is too big!";
}
} else {
echo "There was an error uploading your file" . $fileError . $fileSize;
}
} else if (!(empty(in_array($fileActualExt, $allowed))) && !($allowed)) {
echo "Cannot upload file of this type!";
}
mkdir("channel/" . $username);
mkdir("channel/" . $username . "/videos");
fopen("channel/" . $username . "/index.php", "w");
$account_open_date = date("Y-m-d h:i:s");
$current_date = date("Y-m-d h:i:s");
//$account_open_date_retrieval_sql_select = "SELECT account_open_date from profile0";
//$account_age = date_diff($row, $current_date); // acct open date - current date = account age
//$account_age_result = mysqli_query($conn, $account_open_date_retrieval_sql_select);
//$row = mysqli_fetch_assoc($account_age_result);
// if-then-else-if statement to get rid of the fileDestination var undefined error when avatar photo is not submitted....
if (!(empty($fileDestination))) {
$sqlinsert = "INSERT INTO profiles001 (username, password, email, c_status, doc, avatar, bio) VALUES ('$username', '$hashed_password', '$email', '$confirmation_status', '$account_open_date', '$fileDestination', '$bio')";
} else if (empty($fileDestination)) {
$fileDestination = "assets/soap.jpg";
$sqlinsert = "INSERT INTO profiles001 (username, password, email, c_status, doc, avatar, bio) VALUES ('$username', '$hashed_password', '$email', '$confirmation_status', '$account_open_date', '$fileDestination', '$bio')";
}
$result = mysqli_query($conn, $sqlinsert);
}
}
?>
Hi I have a table class and functions I want to call in another function but can't get it working. Some help will be very welcome. It seesm that the new table class is not working in this function if I pass the values to it, I have tested the class, it does get the post values I post to it so $_POST['id'] are being received as well as all the other $_POST's but the table class and find function is not working, it works fine if I don't put it in a function.. function edit() { if (isset($error)){ $error.="Please fix the error(s) above";} else { if ($_POST['id'] <> "") { $update =& new table($db, 'publisher'); $update->find($_POST['id']); $update->name = $_POST['name']; $update->url = $_POST['url']; $update->contact = $_POST['contact']; $update->address = $_POST['address']; $update->phone = $_POST['phone']; $update->email = $_POST['email']; $update->save(); $error = "The Publisher has been edited"; } } } This will be somewhat of a brain dump with somewhat vague questions. To summarize it right now, something about "refactoring software" has been bugging me. The length of the process, the work it takes, and the time, and the "no immediately visible results" that it produces.
There have been numerous questions on "do I refactor or rewrite". Depending on circumstances, one choice may work better than another, although historically for working projects (projects people are using continuously), refactoring seemed to do better, since it does not break things as much as rewriting does. And when things break all at once, such as when introducing a rewrite (which most likely has different behavior, UI, feel, workflow, etc. etc.), people tend to get upset. Well, refactoring seems to smooth out the 'upsetness' over a much longer period of time. People change as software changes, and each change is not as abrupt as with a rewrite. There are books written about refactoring software. That process takes time and work, `while not changing the functionality of the software`. Little improvements can be done here and there, but the main point becomes improving the internals of the software without affecting the outside behavior, the user experience. From the business end, *there are no changes!*. We have developers *doing busy work* without showing anything to the business, until perhaps much later. Is that bad? Where I work, I have business people scoff at "what have we been doing" quite regularly, and saying things like "add convertion from one unit system to another? Just put it in!" Which ends up me staring into vastness of legacy code and taking 2-3 weeks to "put it in", what in someone's mind takes a day at the most. But that's cuz maybe I'm slow, but I chose to refactor relevant code first, at least moving View items into respective view containers as a first step. What is the purpose of refactoring software? What, per se does it make better? What expectations can be set? What do I do next time someone says "it is taking too long", other than saying ... cuz I am refactoring! Code: [Select] <?php $storedvar['db_table_log']='winning'; class Event1{ function Event2(){ global $storedvar; $this->log = $storedvar['db_table_log']; echo "$this->log"; } } Event1=new Event1; Event1->Event2(); ?> Can check with you how come this coding don't work? Hiya peeps! I was wondering how I could use a function from one class in another EG. class_file_one.php class a { function a() { return true; } } class_file_two.php class b { function b() { if($this->a()) { return 'it works'; } } } The classes are in completely different pages. Many thanks, James. In c++ class we can declare function inside a class and then we can describe that function at bottom. Can i do this in PHP class? In C++ class: class Myclass{ void doall(); } void Myclass::doall(){ ............. ............. } I'm starting to use a class to perform DB functions because I think it is more efficient. I found this class on the internet and understand pretty well what is going on. My problem is that I can't put the results on the screen when I use the class. (I can do it with out using the class but I'd like to learn to use it) function fetch($info) { return mysql_fetch_array($info); this is how I'm attempting to use it $db = new mysql; $db->connect(); while($row = $db->fetch($db->query("SELECT * FROM Attributes"))) { echo $row['Attribute']."<br>"; } This just echos the first row forever and I'm not sure how else to go about this. Any help would be appreciated. Code: [Select] class replayer extends users { --SNIP-- public $islive = false; --SNIP-- function chat($id, $nick, $chat) { --SNIP-- <----------- } } There's my code, I want to take the variable $islive from the above class and use it in the chat() function. How would I do it? I've tried making it a global inside of the chat() function. Hi, I am using a separate class for all the functions. And using using the following function "objectToArray" to convert the JSON decoded object to Array. Class file: class xyz { . . . . . function objectToArray($d) { if (is_object($d)) { $d = get_object_vars($d); } if (is_array($d)) { return array_map(__FUNCTION__, $d); } else { return $d; } } . . . } Main page: $obj = new xyz(); $arr = $obj->objectToArray($json_obj); Because the function "objectToArray" is recursive, array_map(__FUNCTION__, $d) is not able to applies the callback. And throws the warning "array_map() expects parameter 1 to be a valid callback". Need help! Girish can someone help me I am trying to call a class function using a varible.... example say i go to index.php?page=contact i want to pull that class with out doing a switch statement can someone help me Code: [Select] <?php $Site = page; $Site->getPage($_GET['page']); ?> Code: [Select] <?php class page extends site { function getPage($page) { return $this->$page; } function Contact() { echo '<h2>Contact test</h2>'; echo '<form> </form>'; } } ?> Is it possible to pass a callback function to other class's method?
Is what I am attempting to do a bad idea?
Thanks
<?php
class validate
{
public function __construct($data,$callback)
{
//A bunch of script goes here, and I don't want to duplicate it
if($callback) {
//Use $callback function to modify $data
}
}
}
class controller
{
public function savePage()
{
//A bunch of script goes here, and I don't want to duplicate it
$validate=new validate(array('hello'),$this->callback);
}
}
class controller_page1 extends controller
{
public function callback()
{
//script which will be used to modify future $data
}
}
?>
Mon Aug 24, 2020 2:41 pm I'm using php_serial.class.php, called once
require_once("php_serial.class.php");
and call it $serial: $serial = new phpSerial;
The above are in a global.php file that sets up the serial com parameters.
Function checkInput($serial){
And one in the Function: $read = $serial->readPort();
This throws an error: "PHP Fatal error: Uncaught Error: Call to a member function readPort() on string" |
|||||||