Spaghetti Code

Jakebert · November 8, 2009

So.

I've pretty much reached the point where I have so many isset s on one page that I can't tell what the page is supposed to do. In other words: spaghetti code. I just read a few tutorials on OOP, i.e. using classes and such, but is it worth is to re write the entire code with classes, or should I switch to some sort of framework, or should I just keep on going with my tangled ball of death? I'd like to be able to switch to OOP, but I don't really know how to program the thing. Is there a place where someone can help me map out my program so I don't end up like this?

mikesta707 · November 8, 2009

Well to be able to steer you in the right direction, I would have to see some code, but if you are going the OOP route (which is a good idea, in my opinion) remember that OOP works best with multiple classes interacting with each other, instead of one big class that does everything

Jakebert · November 8, 2009

Well, my program is for a school instructors, so if the user needs to be able to:

-Check his classes

-See student info

-Generate report cards for students (already done)

-Look at his reports

-Edit the reports

and another type of user (admin) needs to be able to:

-edit other users

-edit students

-move students from classes

-Organize class times and assign instructors

it's pretty much like a calendar app mixed with a school DB. Which is weird. :shrug:

Then again, maybe I'm over-reacting. But every time I think of a new feature I have to wade through almost every page.

I don't want to give you all the code because there is a lot of it, but here's an example of what edit_class.php looks like right now (i.e. spaghetti):

<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=ISO-8859-1" />
<title>Untitled Document</title>
  
</head>
<body>

<?php

include_once ("auth.php");
include_once ("authconfig.php");
include_once ("check.php");

if ($check['level'] > 4)
	{

print "<font face='Arial' size='5' color='#FF0000'>";
print "<b>Illegal Access</b>";
print "</font><br>";
print "<font face='Verdana' size='2' color='#000000'>";
print "<b>You do not have permission to view this page.</b></font>";
		exit;	// Stop script execution
	}


if (isset($_POST['doneaddingstudent'])){

$fname = $_POST['fname'];
$lname = $_POST['lname'];
$age = $_POST['age'];
$gender = $_POST['gender'];
$level = $_POST['level'];
$class_id = $_POST['class_id'];
$instructor = $_COOKIE['USERNAME'];
echo $fname;
echo $lname;
echo $age;
echo $gender;
echo $level;
echo $class_id;


$sql = mysql_query("INSERT into students VALUES ('', '$fname', '$lname', '$age', '$gender', '$level', '$class_id', '$instructor')");

echo '<h1>Student Added!</h1>';
echo '<a href="list_classes.php">Go back to Classes Lis</a>';

}

else

{

if (isset($_POST['addstudent'])) {

echo '<h1>Add Student</h1>';
?>

<form action = "" method="post">
<?php

$class_id = $_POST['class_id'];

echo 'First Name: <input type = "text" name = "fname" />';
echo '<br />';
echo 'Last Name: <input type = "text" name = "lname" />';
echo '<br />';
echo 'Age: <input type = "text" name = "age" />';
echo '<br />';
echo 'Male: <input type = "radio" name = "gender" value = "male"/>';
echo 'Female: <input type = "radio" name = "gender" value = "female"/>';
echo '<br />';
echo 'Level: <input type = "text" name = "level" />';
echo '<br />
<input type = "hidden" name = "class_id" value = "'.$class_id.'">
<button type=button value=Cancel onClick="javascript:history.go(-1);">Back</BUTTON>
<input type = "submit" name = "doneaddingstudent" value = "Add Student" />

</form>';


}



$class_id = $_POST['class_id'];
$new_level=$_POST['new_level'];
$new_day=$_POST['new_day'];
$new_time=$_POST['new_time'];
$new_session=$_POST['new_session'];
$new_instructor=$_POST['instructor'];

if (isset($_POST['submit']))
{


$query= ("UPDATE classes SET level='$new_level', day='$new_day', time='$new_time', session='$new_session' WHERE id='$class_id'");

echo $query;

$sql = mysql_query($query) or die(mysql_error());

}

else

{

$class_id=$_POST['class_id'];
$level=$_POST['level'];
$day=$_POST['day'];
$time=$_POST['time'];
$session=$_POST['session'];
$new_instructor=$_POST['instructor'];

echo '<table>
<form action = "" method="post">
<input type = "hidden" name = "class_id" value = "'.$class_id.'" />
<tr><td>
<b>ID:</b></td>
<td>'; 
echo $class_id;
echo '</td>
<tr><td>
<b>Level</b>:</td>
<td><input type = "text" name = "new_level" value="'.$level.'" />
</td></tr>';

echo '</td>
<tr><td>
<b>Day</b>:</td>
<td><input type = "text" name = "new_day" value="'.$day.'" />
</td></tr>';

echo '</td>
<tr><td>
<b>Time</b>:</td>
<td><input type = "text" name = "new_time" value="'.$time.'" />
</td></tr>';

echo '</td>
<tr><td>
<b>Session</b>:</td>
<td><input type = "text" name = "new_session" value="'.$session.'" />
</td></tr>';

echo '</td>
<tr><td>
<b>Instructor</b>:</td>
<td><select name = "new_instructor" value="'.$instructor.'" />
<option value = "'.$instructor.'" selected = "selected">"$instructor</option>';


$query1 = ("SELECT * authuser");
$sql1 = mysql_query($query1) or die(mysql_error());

while ($row = mysql_fetch_array($sql1)) {
echo '<option value = "\'.$row[\'instructor\'].\'">$row[\'instructor\']</option>';

echo '</td></tr>

<button type=button value=Cancel onClick="javascript:history.go(-1);">Cancel</BUTTON>
<input type="submit" name="submit" value = "Edit Attributes" /></form>';
}

echo '</table>';

echo '<br /><br /><h3>Students</h3>';


$query = ("SELECT * FROM students WHERE classid = '$class_id'");
$sql = mysql_query($query) or die(mysql_error());

if (mysql_num_rows($sql) == 0)
{
echo "No students in class";
}

else

{
echo '<table border = "1"><tr><td>ID</td><td>First</td><td>Last</td><td>Age</td><td>Level</td></tr>';
while ($row = mysql_fetch_array($sql))

{
	echo '<tr><td>';
	echo $row['id'];
	echo '</td><td>';
	echo $row['fname'];
	echo '</td><td>';
	echo $row['lname'];
	echo '</td><td>';
	echo $row['age'];
	echo '</td><td>';
	echo $row['level'];
	echo '</td><td>';
	echo '<form action="edit_student.php" method = "get">';
	echo '<input type = "hidden" name = "sid" value = "'.$row['id'].'" />';
	echo '<input type = "submit" name = "submit2" value = "Edit" /></form>';
	echo '</td></tr>';
}
}

echo '<form action = "" method="post">';
echo '<input type = "hidden" name = "class_id" value = "'.$class_id.'")';
echo '<input type = "submit" name = "addstudent" value = "Add Student" /></form>';

echo '</table>';


}
}
?>

</body>
</html>

mikesta707 · November 8, 2009

I've seen worse, but an OOP style may help you. I would start with a student class (controls the students attributes, like grades and such, and updates them), and maybe a class (as in school class) class (that has an array of the students and stuff).

Jakebert · November 8, 2009

OK, so let's say we want to add a student. Would you put the student class on a separate file and then include it in the file that the user sees (the one with the form?) or do you put them together in one big file?

mikesta707 · November 8, 2009

I would suggest putting each class (or groups of related classes) in their own include files.

Jakebert · November 8, 2009

I'm sorry I'm being such a brat about this. I hope I get the hang of it soon.

So I have one file, for now it will hold my PHP classes:

<?php
// PHP 5

// STUDENT class
class Student {
public $sid;
public $fname;
public $lname;
public $gender;
public $bday;
public $level;

// define methods

public function move() {
	echo 'This is a TEst';
}

public function edit() {
}

public function add($fname) {
	$new_student->fname = $_POST['fname'];
	echo $fname;
}

}

Here is the page the user sees:

<?php
// PHP 5
// Test
include 'connect.php';
include 'bizlog.php';

mysql_connect($host,$user,$pass);
mysql_select_db($db);

if(isset($_POST['submit'])) {

$new_student = new Student;
$new_student->add($fname);

}

else

{
echo '<form action = "" method = "post">
<input type="text" name = "fname" />
<input type="submit" name = "submit" />
</form>'; 
}

?>

I'm getting a blank page after I hit submit. Shouldn't it be echoing the name?

mikesta707 · November 8, 2009

this

if(isset($_POST['submit'])) {

$new_student = new Student;
$new_student->add($fname);

}

doesn't echo anything, so no you shouldn't be getting any output on the screen.

By the way, instead of creating an add function that will set the $fname attribute, I would just create a constructor that does that (since naming the student should be done when the object creation. THere is nothing stopping someone from creating a nameless student)

Jakebert · November 8, 2009

But then what goes in the class methods? If I put mySQL updates and assigning of variables in the front page, then what goes into the class methods? Won't I just be recreating my old spaghetti code all over again?

cbearhoney · December 2, 2009

Jakebert,

Did you get to finish this project? I'm moving from procedural to OOP programming but I'm a little lost in how to organize my code. I thought your project was a good one to follow.

KevinM1 · December 2, 2009

Jakebert,

Did you get to finish this project? I'm moving from procedural to OOP programming but I'm a little lost in how to organize my code. I thought your project was a good one to follow.

Feel free to make a new thread if you have questions.

Cardale · December 2, 2009

OOP is only good if your working in a group or have bad organizational skills. "Spaghetti" code is much faster. Although your spaghetti could use some organization. OOP is more about predefined constructs to keep you in shape. "Spaghetti" is more about personal constructs to keep your self in shape. At least thats the way I see it many others will probably see it different.

KevinM1 · December 2, 2009

OOP is only good if your working in a group or have bad organizational skills. "Spaghetti" code is much faster. Although your spaghetti could use some organization. OOP is more about predefined constructs to keep you in shape. "Spaghetti" is more about personal constructs to keep your self in shape. At least thats the way I see it many others will probably see it different.

:facepalm:

Cardale · December 2, 2009

an explanation would of been better because honestly you don't need classes at all. Heck you don't need these high level languages. You could use assembly.

KevinM1 · December 2, 2009

an explanation would of been better because honestly you don't need classes at all. Heck you don't need these high level languages. You could use assembly.

Procedural code may be faster than OO code in terms of cycles, depending on how objects are created/disposed behind the scenes. That said, OO code tends to make writing and maintaining code much faster.

By its nature, OO code is modular. Meaning, a change in one portion of the system won't likely necessitate a change in another portion.

This modularity also promotes code reuse. Instead of copy-and-pasting individual lines of code from one project that may work in another, you can transport entire subsystems without much effort.

These two benefits reduce the time it takes to actually write useful code. It also makes site maintenance much easier.

Cardale · December 2, 2009

This is true in the sense that it creates this "ease" for you. It is just a control structure. You can design this control structure yourself. I am not sure what you mean by change in multiple places because if you do it right you can do the same thing with the later. OOP is nothing more than an idea a constructed human fabrication.

People also say it is more modular. I don't understand this. If you have a login script for example in one file it could be very easy to use it in another system if that is your goal if it is a class or isn't

Sign In

Spaghetti Code

Recommended Posts

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Join the conversation

Important Information