views:

64

answers:

2

Hi there

I would like to list my portfolio on our website, and I wrote a class, Client for setting and getting parameters about each client's basics, i.e.: the client's name, website, is it featured, is it published, what items did we do for them, etc.

It's fairly primitive, but am I correct in assuming it's better to create an object for each client (and keeping each's data together) instead of creating array's for each parameter and calling corresponding elements from these array's everytime?

Here's my class, I know it's still primitive, but I might create some other functions as well, so for now I'm just using a basic getter and setter to get or set parameters for each Client.

<?php

class Client {
    private $name = "";
    private $items = array();
    private $website = "";
    private $featured = false;
    private $published = false;
    private $type = array();
    private $filename = null;
    private $extension = null;

    public function __set($name, $value) {
        //echo "Setting ".$name." to value: ".$value."\n";
        $this->{$name} = $value;
    }

    public function __get($name) {
        //echo "Getting ".$name.": ".$this->{$name};
        return $this->{$name};
    }

    public function __isset($name) {
        return isset($this->{$name});
    }

    public function __unset($name) {
        unset($this->{$name});
    }
}

?>

I am still planning the functions I might want to add to the class, but for now this is it.

Here is my other code I'm using to create each Client object:

<?php

// create Client object for every client

$files = array();
// files to take out of file listing, I'm developing on Mac, i.e. ._DS_Store file
$bad_files = array(".","..","._DS_Store");
$portfolio = "portfolio";
$images = "images";
$details = "details";
$thumbs = "thumbs";

// get all *.txt files listed in portfolio, so a client will not be added to portfolio without the necessary details.
if (is_dir("$images/$portfolio")) {
    if (is_dir("$images/$portfolio/$details")) {
        $files = scandir("$images/$portfolio/$details");

        sort($files);
    }
}
$files = array_diff($files, $bad_files);
sort($files);

// keeps a list of all clients
$clients = array();

foreach ($files as $file) {
    $value = file_get_contents("$images/$portfolio/$details/$file");

    $newClient =  new Client();

    $filename = explode(".",$file);
    $newClient->filename = $filename[0];
    $client_image = glob("$images/$portfolio/$images/".$newClient->filename.".*");
    $newClient->image = $client_image[0];
    $client_thumb = glob("$images/$portfolio/$thumbs/".$newClient->filename.".*");
    $newClient->thumb = $client_thumb[0];

    $client_items = array();
    $client_type = array();

    // gets variables from text file contents and explode string to array [key=value] values
    $content = explode("&",$value);
    for ($j=0; $j<count($content); $j++) {
        $client = explode("=", $content[$j]);
        if (strpos($client[0],"name") !== false) $newClient->name = $client[1];
        if (strpos($client[0],"items") !== false) $client_items = $client[1];
        if (strpos($client[0],"website") !== false) $newClient->website = $client[1];
        if (strpos($client[0],"featured") !== false) $newClient->featured = $client[1]; // show on frontpage
        if (strpos($client[0],"published") !== false) $newClient->published = $client[1]; // show at all
        if (strpos($client[0],"type1") !== false) $client_type[] = $client[1]; // show for specific type, eg. business card, website
        if (strpos($client[0],"type2") !== false) $client_type[] = $client[1]; // show for specific type, eg. business card, website
    }

    // these parameters need array type values
    $newClient->type = $client_type;
    $newClient->items = explode(", ",$client_items);

    // adds client to list of clients
    $clients[] = $newClient;
}

?>

Here is the code I'm using to output each client's banner and details:

<div id="banner_content">
    <?
        foreach ($clients as $client) {
        // client must be published to show at all
            if ((($page == "home" && $client->featured) || $page != "home") && $client->published) {
    ?>
    <div class="banner_container"><img src="<? echo $client->image; ?>" width="809" height="324" alt="<? echo $client_name; ?>" title="<? echo $client_name; ?>" />
        <div class="banner_details">
            <div class="client_name">Client: <b><? echo (!empty($client->name) ? $client->name : "Unknown"); ?></b></div>
            <div class="client_items"><? echo (!empty($client->items) ? "Items: <b>".join(", ",$client->items)."</b>" : ""); ?></div>
            <div class="client_website"><? echo (!empty($client->website) ? "Website: <b><a href=\"http://".strtolower($client-&gt;website)."\"&gt;".$client-&gt;website."&lt;/a&gt;&lt;/b&gt;" : ""); ?></div>
        </div>
    </div>
    <?
    }
}
?>
</div>

Any help or advice would be much appreciated. Thanks in advance.

// edit

I forgot to mention, I actually wrote the class, because there will be a Portfolio page and which will contain more info about the client than just the above mentioned info. I know a class is a bit overkill for just listing images in a banner.

A: 

I don't have any experience with web development, but using a class here is definitely better programming practice.

Managing multiple parallel arrays is troublesome. For instance, whenever you insert a new item or need to move items, you need to remember each array. Furthermore, using a class here makes your code more organized, and hence more maintainable.

I think it is certainly worth the overhead of implementing the class, especially once you've got the hang of using classes in PHP!

Bernhard Häussermann
Aaahhh, dankie Bernhard!
Anriëtte Combrink
Is it ok to keep track of the multiple Client objects via an array then?
Anriëtte Combrink
A: 

If a class is an effective solution depends on how you implement it. It's not like your code would magically be enhanced because you slap a class declaration on something.

One of the key ideas of using classes is to encapsulate state and responsibility. So having a Class Client that encapsulates Client-related state is a good idea. Giving it methods to render itself as a List or Table is not. I know, you are not doing this, but you are also not using a class that has the responsibility to render a Client into a List or Table. Nor do you have classes to find the related Client Files. Your second posted codeblock is a procedural script. It does not use OOP. Identify Concerns and break them down into small (testable) units. Merge them into appropriate classes.

There is a nice series of articles by Lorna Jane Mitchell about PHP5 and OOP and there is a number of good Q&A on StackOverflow as well. You also wanna have a look at what Design Patterns are. And here is a shameless self-plug.

Gordon