Im trying to imprive my skills and move away from procedural style coding, however the switch is not coming easy and I am very much new to the OOP concept so please keep this in mind when reading / answering.
I have the following method inside a class called Jobs{}
The task of the method is pretty simple query the database for all the different job categories in a table and return it.
The following is an extract from the code inside my class
function dispCategories(){
$sql = "SELECT DISTINCT category FROM jobs";
$stmnt = $db->prepare($sql);
$stmnt->execute();
return $stmnt->fetchAll();
}
Now inside my view or html page I do the following
$obj = new Jobs();
$categories = $obj->dispCategories()
?>
<!--DISPLAY CATEGORIES -->
<form id="getJobByCategory" method="post" action="">
<select name="selectedCategory">
<?php
foreach ($categories as $category){
?>
<option value="<?php echo $category['category'] ?>">
<?php echo $category['category'] ?>
</option>
<?php
}
?>
My Question
When you look at my code where I initiate the object Jobs{} and call the method categories $categories = $obj->dispCategories()
I use a foreach loop combined with some html & php. Is this an acceptable way of doing things in the OOP realm?
getCategories() where I loop over the results returned by the dispCategories() method? dispCategories() method inside the actual dispCategories() method...Hope this make sense. Just looking for some best practices and guidance here, however please keep in mind im a rookie.
Should I rather create a seperate method like getCategories() where I loop over the results returned by the dispCategories() method?
You're definitely on the right track with what you have now. Since your dispCategories() method only returns one column, it makes sense that it would return a one-dimensional array, a simple list of the values rather than a 2D structure that retains characteristics of the SQL query that generated it. I would probably modify the method to fetch all rows and then reduce them to a single array, which you then return. This makes it unnecessary to create another method just for looping over the categories returned by dispCategories().
Assuming PHP 5.5+, you can easily do this with array_column():
function dispCategories(){
$sql = "SELECT DISTINCT category FROM jobs";
$stmnt = $db->prepare($sql);
$stmnt->execute();
// Return a simple 1D array of the category only
return array_column('category', $stmnt->fetchAll());
}
It's then a simple foreach to list them out without needing to access array keys.
foreach ($categories as $category){
?>
<option value="<?php echo $category; ?>">
<?php echo $category; ?>
</option>
<?php
}
Remember that you may need to call htmlspecialchars() on the $category value if it may contain characters requiring HTML entity encoding.
Alternatively should I just process the data returned by the dispCategories() method inside the actual dispCategories() method...
No, I don't think you ought to do this. As you learn OOP, read up on the concept of separation of concerns and on the MVC (model-view-controller) pattern. If you were to create a method in your Jobs class that created this HTML block, it would then be performing work that is more appropriately done by your view code. It should be the responsibility of the Jobs class to retrieve items from the database, provide information about them, and prepare them to be passed to a view or template, but the view or template HTML should retain responsibility for organizing a list of categories from the Jobs class into markup appropriate for however you happen to be displaying it.
If you love us? You can donate to us via Paypal or buy me a coffee so we can maintain and grow! Thank you!
Donate Us With