/    Sign up×
Community /Pin to ProfileBookmark

php critique needed

my first few sites in php were ugly as all hell. i slowly moved towards organization, so now each page was more organized because i started using functions.

i have taken the next step and now i use OOP to keep the bulk of my code on 1 include file. what i need some help is critiquing my OOP design to see if i can be more effecient or not.

the classes.inc file, the file with all my classes, is about 600 lines of code and i feel reluctant to just throw that on there.

instead, here is a segment of it.. i can (and would like to) post the rest if possible, please let me know, thanks.

[code=php]class Articles
{
var $Title;
var $Something;
var $CategoryName;

function GetCategories() //a while loop is set up on the page that calls
{
$query = “SELECT * FROM Categories”;
return mysql_query($query);
}

function SetCategoryName($CategoryName_p)
{
$this->CategoryName = $CategoryName_p;
}

function GetArticles($CategoryID) //another while loop setup on the page using this
{
$query = “SELECT a.* FROM Articles a, ArticleCategories ac, Categories c
WHERE c.CategoryID = ac.CategoryID
AND ac.CategoryID = $CategoryID
AND a.ArticleID = ac.ArticleID
AND a.Approved = 1″;
return mysql_query($query);
}

function PostArticle($Title_p, $Body_p, $CategoryID_p)
{
$CheckFields = Array(‘Title’ => $Title_p,
‘Body’ => $Body_p);
if($_SESSION[‘Handler’]->CheckFields($CheckFields)) //check fields is a function that uses a for each loop to check every item, return true if not empty false otherwise
{
$query = “INSERT INTO Articles
(Title, Body, UserID, Approved)
VALUES
(‘$Title_p’, ‘$Body_p’, “.$_SESSION[‘LoginSession’]->UserID.”, 0);”;
if(mysql_query($query))
{
$_SESSION[‘Handler’]->SetMessage(“Your article has been successfuly submitted”, false); //handler is a session instance of a class that sets function-wise messages.. i.e. invalid login info, comment submited, etc.
$this->NotifyUser($Title_p);
$this->NotifyAdmin(mysql_insert_id());
}
else
$_SESSION[‘Handler’]->SetMessage(“There was a problem submitting your article, please try again later”, true);
header(‘Location: post_article.php’);
exit; //i have to exit after using header so the ShowMessage() is not called, doing so erases the message set 2 lines above
}

/*$ArticleID = mysql_insert_id();

$query = “INSERT INTO ArticleCategories
(ArticleID, CategoryID)
VALUES
($ArticleID, $CategoryID)”;
mysql_query($query);*/
}

function NotifyUser($Title_p)
{
$Message = “Thank you. Your contribution for article $Title_p has been submitted and upon approval will be posted momentarily. Feel free to contact us with any additional questions. “;
$UsersEmail = $_SESSION[‘LoginSession’]->EmailAddress;
$Subject = “Article Submission”;
$FromEmail = “[email protected]”;
$Headers = “From: [email protected]”;
$Headers .= “Content-type: text/htmlrn”;
@mail($UsersEmail, $Subject, $Message, $Headers);
}

function NotifyAdmin($ArticleID_p)
{
$Message = “A new article has been submitted. See Article #$ArticleID_p to approve. If approved, add categories to the article”;
$AdminEmail = “[email protected]”;
$Subject = “Article Submission”;
$FromEmail = “[email protected]”;
$Headers = “From: [email protected]”;
$Headers .= “Content-type: text/htmlrn”;
@mail($UsersEmail, $Subject, $Message, $Headers);
}

function ArticleInfo($ArticleID_p)
{
$query = “SELECT CONCAT(u.Firstname , ‘ ‘ , u.LastName) AS Name, a.*, DATE_FORMAT(DateSubmitted, ‘%c-%d-%Y’) AS DateSubmitted
FROM Articles a, Users u
WHERE ArticleID = $ArticleID_p
AND u.UserID = a.UserID”;
$result = mysql_query($query);
return mysql_fetch_array($result);
}

function InsertArticleComment($Rating_p, $Comments_p, $ArticleID_p)
{
$CheckFields = Array(‘Rating’ => $Rating_p,
‘Comments’ => $Comments_p);
if($_SESSION[‘Handler’]->CheckFields($CheckFields))
{
$query = “INSERT INTO ArticleComments
(Rating, Comment, ArticleID, UserID)
VALUES
($Rating_p, ‘$Comments_p’, $ArticleID_p, “.$_SESSION[‘LoginSession’]->UserID.”)”;
if(mysql_query($query))
$_SESSION[‘Handler’]->SetMessage(“Your comment has been successfully posted”, false);
else
$_SESSION[‘Handler’]->SetMessage(“There was an error posting your comment”, true);
header(“Location: add_comment_article.php?ArticleID=$ArticleID_p”);
exit;
}
}

function ShowArticleComment($ArticleID_p)
{
$query = “SELECT ac.*, u.FirstName, u.AccessLevel
FROM ArticleComments ac, Users u
WHERE ac.ArticleID = $ArticleID_p
AND u.UserID = ac.UserID
ORDER BY DateSubmitted DESC”;
return mysql_query($query);
}
/*END REVIEW
}
[/code]

to post a comment
PHP

1 Comments(s)

Copy linkTweet thisAlerts:
@ixxalnxxiauthorJun 11.2008 — here's also what i do not know:

although i've never used, i've been a big fan of the MVC but have not found any small framework that supports php4 also.. so i like to keep display away from logic as much as possible.

this being said, i feel as if i have to put too much code onto the pages that contain the html.

i also find my self wondering about small things, like what the standard is with letter case in variable/function names.

if anyone has general OOP design / MVC tips for a newbie, let me know.

p.s. i come from a background of C++/Java (only as far as a college Data Structures course though) so i'm familiar with some lingo and data structure concepts)
×

Success!

Help @ixxalnxxi spread the word by sharing this article on Twitter...

Tweet This
Sign in
Forgot password?
Sign in with TwitchSign in with GithubCreate Account
about: ({
version: 0.1.9 BETA 5.18,
whats_new: community page,
up_next: more Davinci•003 tasks,
coming_soon: events calendar,
social: @webDeveloperHQ
});

legal: ({
terms: of use,
privacy: policy
});
changelog: (
version: 0.1.9,
notes: added community page

version: 0.1.8,
notes: added Davinci•003

version: 0.1.7,
notes: upvote answers to bounties

version: 0.1.6,
notes: article editor refresh
)...
recent_tips: (
tipper: @AriseFacilitySolutions09,
tipped: article
amount: 1000 SATS,

tipper: @Yussuf4331,
tipped: article
amount: 1000 SATS,

tipper: @darkwebsites540,
tipped: article
amount: 10 SATS,
)...