patternphpMinor
Blog engine or somesuch
Viewed 0 times
blogenginesomesuch
Problem
Would someone tell how I can improve this piece of code?
```
per_page = $per_page;
}
function set_total_page($total) {
$this->totalPage = $total;
}
function set_page($page) {
$this->page = $page;
}
function get_page() {
return $this->page;
}
function get_per_page() {
return $this->per_page;
}
function get_total_page() {
return $this->totalPage;
}
function counts($cats) {
$sql = "select count(post_id) from Post where category='$cats'";
$result = $this->query($sql);
$rows = mysqli_fetch_row($result);
return $rows;
}
function cats() {
$sql = "select distinct category from Post ";
$result = $this->query($sql);
while ($rows = mysqli_fetch_array($result)) {
$row[] = $rows;
}
mysqli_free_result($result);
$this->closeDb();
return $row;
}
function postByCats($cats) {
if ($cats == NULL)
return $this->Posts($cats);
$sql = "select * from Post where category='$cats'";
$result = $this->query($sql);
while ($row = mysqli_fetch_assoc($result))
$tuts[] = $row;
mysqli_free_result($result);
$this->closeDb();
return $tuts;
}
function Posts($many = NULL) {
$sql = "select count(post_id) from Post";
$per = $this->set_per_page(3);
$query = $this->query($sql);
$num = mysqli_fetch_row($query);
$numrows = $num[0];
if (isset($_GET['page']) && is_numeric($_GET['page'])) {
$num = intval($_GET['page']);
$this->set_page($num);
} else {
$num = 1;
$this->set_page($num);
}
//offset and number per page
$total = ceil($numrows / $this->get_per_page());
$this->set_total_page($total);
$num_off = $this->get_per_page();
if ($this->get_page() > $this->get_tota
```
per_page = $per_page;
}
function set_total_page($total) {
$this->totalPage = $total;
}
function set_page($page) {
$this->page = $page;
}
function get_page() {
return $this->page;
}
function get_per_page() {
return $this->per_page;
}
function get_total_page() {
return $this->totalPage;
}
function counts($cats) {
$sql = "select count(post_id) from Post where category='$cats'";
$result = $this->query($sql);
$rows = mysqli_fetch_row($result);
return $rows;
}
function cats() {
$sql = "select distinct category from Post ";
$result = $this->query($sql);
while ($rows = mysqli_fetch_array($result)) {
$row[] = $rows;
}
mysqli_free_result($result);
$this->closeDb();
return $row;
}
function postByCats($cats) {
if ($cats == NULL)
return $this->Posts($cats);
$sql = "select * from Post where category='$cats'";
$result = $this->query($sql);
while ($row = mysqli_fetch_assoc($result))
$tuts[] = $row;
mysqli_free_result($result);
$this->closeDb();
return $tuts;
}
function Posts($many = NULL) {
$sql = "select count(post_id) from Post";
$per = $this->set_per_page(3);
$query = $this->query($sql);
$num = mysqli_fetch_row($query);
$numrows = $num[0];
if (isset($_GET['page']) && is_numeric($_GET['page'])) {
$num = intval($_GET['page']);
$this->set_page($num);
} else {
$num = 1;
$this->set_page($num);
}
//offset and number per page
$total = ceil($numrows / $this->get_per_page());
$this->set_total_page($total);
$num_off = $this->get_per_page();
if ($this->get_page() > $this->get_tota
Solution
If the code will be distributed, read or maintained by others, you might want to look at your naming conventions as they're a bit inconsistent, for example sometimes you use underscore separated names such as
Aside from your single line getters and setters, the function names you've chosen are not always indicative of what the function does. Ideally you shouldn't have to read the actual code to know what a function does, consider these changes:
You also haven't got any access modifiers on your methods. As
As for the actual code, aside from collapsing some statements there's not much to change. These lines in
In addition to the empty
get_per_page(), but other times you use sulkingCamelCase such as postByCats() and Posts() in class case. Either one is fine, as long as you use the same throughout.Aside from your single line getters and setters, the function names you've chosen are not always indicative of what the function does. Ideally you shouldn't have to read the actual code to know what a function does, consider these changes:
function counts($cats)
function getNumberOfPostsForCategory($category)
function cats()
function getCategories()You also haven't got any access modifiers on your methods. As
paginate() directly prints out a segment of HTML which wouldn't be valid on its own, I'm guessing that it's probably only used once somewhere in your View and as such should be private or protected to prevent misuse.As for the actual code, aside from collapsing some statements there's not much to change. These lines in
paginate() don't appear to be doing anything though:$sql = 'select count(post_id) from Post';
$query = $this->query($sql);In addition to the empty
else statement in the Posts() method.Code Snippets
function counts($cats)
function getNumberOfPostsForCategory($category)
function cats()
function getCategories()$sql = 'select count(post_id) from Post';
$query = $this->query($sql);Context
StackExchange Code Review Q#51875, answer score: 2
Revisions (0)
No revisions yet.