another OOP question

[Avido]

Hello,

i've recently started to use OOP. I read the chapter in The PHP Anthology and I came up the following classes for handeling newsitems(3 languages, dutch, french and english):

Some translations:
feit = fact
titel = title

file: newsItem.php

<?php
 class newsItem {
 
  var $id;
  var $dt; //date
  var $titel_nl;
  var $feit_nl;
  var $titel_fr;
  var $feit_fr;
  var $titel_en;
  var $feit_en;
  
  function newsItem($data) {
   $this->id=$data['nieuwsID'];
   $this->dt= $data['dt'];
   $this->titel_nl = stripslashes($data['titel_nl']);
   $this->feit_nl = stripslashes($data['feit_nl']);
   $this->titel_fr = stripslashes($data['titel_fr']);
   $this->feit_fr = stripslashes($data['feit_fr']);
   $this->titel_en = stripslashes($data['titel_en']);
   $this->feit_en = stripslashes($data['feit_en']);
  }
  
  function id() {
   return $this->id;
  }
  
  function dt() {
   return $this->dt;
  }
  
  function titel_nl(){
   return $this->titel_nl;
  }
  
  function feit_nl() {
   return $this->feit_nl;
  }
  
  function titel_fr(){
   return $this->titel_nl;
  }
  
  function feit_fr() {
   return $this->feit_fr;
  }
  
  function titel_en(){
   return $this->titel_nl;
  }
  
  function feit_en() {
   return $this->feit_en;
  }
  
 }
  
?>

file: news.php

<?php
 require_once 'newsItem.php';
 
 class news {
 
  var $db;
  var $n_items;
  var $nrOfFacts;
    
  function nieuws(&$db){
   $this->db = &$db;
  }
  
  function getNews() {
   
   $sql = "SELECT
      nieuwsID, DATE_FORMAT(datum, '%d.%m.%Y') as dt, titel_nl, 
      feit_nl, titel_fr, feit_fr, titel_en, feit_en
     FROM
      tblNieuws
     ORDER BY
      datum DESC";
   
   $result = $this->db->query($sql);
   $this->nrOfFacts = $result->size();
   
   if($result->isError()) {
    trigger_error('nieuws::getNieuws: Unable to fetch news');
    return false;
   }
   
   while($row = $result->fetch()) {
    $this->n_items[] = new newsItem($row);
   }
        
   return true;
  }
?>

First of all, is this a suitable way?
And second, how can I implement a function to insert a new newsItem in the database?

tia

[Big Fat Bob]

Yo

The second script example look okay, though not the first I'm afraid

For example, your class supports 3 languages, but what happens if you need a Norwegian translation ? Well, you'd have to change the class.

This isn't the best approach in two ways;

1) You have all translations within the same class, I'd have a base class, and extend each translation, and
2) Even though I don't have a solution for you, I'm getting a feeling from reading another related thread that using OO as a content container isn't ideal.

Anyway, onto your second question, you could use a Gateway pattern for this, for example have a look at this script posted in this thread for an example;

http://www.sitepoint.com/forums/showthread.php?t=186476

Hope this helps you

[arborint]

My personal opinion is that classes like your newsItem are a crazy use of OO in PHP. Others will disagree, but I think associative arrays are better than OO in mamy cases. Simpler would be just one class:

 class news {



  var $db;

  var $n_items;

  var $nrOfFacts;

    

  function news(&$db){

   $this->db = &$db;

  }

  

  function fetchNews() {

   

   $sql = "SELECT

      nieuwsID, DATE_FORMAT(datum, '%d.%m.%Y') as dt, titel_nl,

      feit_nl, titel_fr, feit_fr, titel_en, feit_en

     FROM

      tblNieuws

     ORDER BY

      datum DESC";

   

   $result = $this->db->query($sql);

   $this->nrOfFacts = $result->size();

   

   if($result->isError()) {

    trigger_error('nieuws::getNieuws: Unable to fetch news');

    return false;

   }

   

   while($row = $result->fetch()) {

    $this->n_items[$row['id']] = $row;

   }

        

   return true;

  }



function getItem($id, $lang)

{

  return ($this->n_items[$id][$lang]);

}



| 

If the newsItem class was doing more it might make sense, but I might still use an array lookup rather than a bunch of accessor functions.

[DougBTX]

Originally Posted by arborint

My personal opinion is that classes like your newsItem are a crazy use of OO in PHP.

Is it even OO??

Looks like the class is being used as a record rather than an object (in the OO sense), in which case an array is much better.

Douglas

[McGruff]

There's nothing wrong with using a class as a container - see WACT use of dataspaces. If you were storing lots of different types of information in an array, you might think about an object instead. Rather than a messy multi-dimensional array structure each type can have its own property.

[DougBTX]

I wouldn't advocate an object or a multi-dimensional array here.

Why is data for all the languages being pulled from the database?

If fetchNews() was called at fetchNews($lang) the returned data could be simplified to an array of 'nieuwsID', 'dt', 'feit' and 'titel'. This could be done in the SQL to save processing later:

SELECT titel_$lang titel, feit_$lang feit

The while loop could be simplified:

function striprow($row) { // This function replaces the newsItem class!

   $row['titel'] = stripslashes($data['titel']);

   $row['feit'] = stripslashes($data['feit']);

}



   while($row = $result->fetch()) {

    $this->n_items[] = striprow($row);

   } 

Douglas

[arborint]

I agree with Douglas that this code could be simplified considerably. We'd need to know the bigger picture to know how to do that though.

There's nothing wrong with using a class as a container - see WACT use of dataspaces.

In languages with powerful, build-in associative arrays like PHP, using a class as an array makes less sense than with other languages. WACT dataspaces are a pretty sophisticated concept; not sure if it applies here.

If you were storing lots of different types of information in an array, you might think about an object instead.

As PHP is typeless this is less important than in typed languages where the vars actually need very different code to handle them or you get errors. However, ff the data has different operations on the vars it starts to make sense to use an opject (e.g. an Iterator one, a string operation on another).

Rather than a messy multi-dimensional array structure each type can have its own property.

I don't consider multi-dimensional arrays messy. The are a very efficient way to store data and map well onto databases.

[Avido]

All the languages are taken from the database at the same time because i use this for a content management system.
So, our site is can be viewed in three different languages, but if we want to insert a new newsitem, we have to provide it in all the different languages.

[arborint]

Yes, but you only view the whole site in one language at a time. Therefore you can have a single global setting for the language. Then when you fetch an article you will only get the id, date and content for the current language. It greatly simplifies your other code because it only needs to look for $row['title'].

[Big Fat Bob]

Yo

Off topic slightly but has anyone thought about Web Services yet ?

[DougBTX]

Originally Posted by Big Fat Bob

Yo

Off topic slightly but has anyone thought about Web Services yet ?

Too off topic for me! Can you give me some hints as to what you are talking about??

Do you mean "perhaps he is serving a web service which must deliver all the content in three languages for the clients to make the lang choice"?

Douglas

[Radley]

At the very least stick them in an array and use something like:

$item = new NewsItem('en');

echo $item->titel(); 

[lazy_yogi]

For multiple languages, you might like to try out the strategy pattern. It would make adding more languages much easier, as well as improve code readability and ease maintainance time

class NewsItem
{
  var $id;
  var $date;
  var $title;
  var $fact;
 
  function setRow($id, $date, $title, $fact) 
  {
    // implement this
  }
 
  // getters and setters here
}
 
class EnglishNewsItem extends NewsItem
{
  function EnglishNewsItem($data)
  {
    setRow($data['nieuwsID'], $data['dt'], stripslashes($data['titel_en']), stripslashes($data['feit_en']));
  }
}
 
class FrenchNewsItem extends NewsItem
{
  function FrenchNewsItem($data)
  {
    setRow($data['nieuwsID'], $data['dt'], stripslashes($data['titel_fr']), stripslashes($data['feit_fr']));
  }
} 

class News
{
  var $db;
  var $n_items;
  var $nrOfFacts;
 
  function News(&$db){
    $this->db = &$db;
  }
 
  function getNews() 
  { 
     ....
    while($row = $result->fetch())
      $this->n_items[] = createNewsItem($row);
  }
 
  // abstract method to implement in subclasses - see below
  function createNewsItem($row) { }
}
 
class EnglishNews extends News
{
  function createNewsItem($row)
  {
    return new EnglishNewsItem($row);
  }
}
 
class FrenchNews extends News
{
  function createNewsItem($row)
  {
    return new FrenchNewsItem($row);
  }
} 

$news = new EnglishNews($db);
$news->getNews();
 
// OR 
 
$news = new FrenchNews($db);
$news->getNews(); 

It probably doesnt run as I havent tested it out, but you get the idea. This way it is very easy to add languages.

[firepages]

I think you really need to look at DougBTX's first post , there is no reason nor justification for pulling all the different language data from the DB if you only need the currently requested language, (of course you may need several languages at once but please note if so).

once the sql calls are dynamic then you can lose some of your method bloat as well (and easily add new languages)

<?
function titel( $lang ){
$var = 'titel_' . $lang ;
return $this->$var ;
}
?>

but any discussion past the sql point is not relevant until you sort that out.

[lazy_yogi]

That's trivial to solve. You do it exactly the same way - create an abstract method to subclass and put any language specific code there, and call that method from the super class.

Just have another abstract method in classes that subclass News, and put in something like

class EnglishNews extends News
{
  function columns()
  {
     return " titel_en, feit_en ";
  }
} 

And in his News super class getNews method, change it to this:

$sql = "SELECT
     nieuwsID, DATE_FORMAT(datum, '%d.%m.%Y') as dt, ".$this->columns()."
     FROM
     tblNieuws
     ORDER BY
     datum DESC"; 

[firepages]

I know its simple to solve , (I dont think your example is that solution since you still end up having to create new classes for each language where a simple bit of variable substitution would suffice ($this->lang)) the point is is that is where you should be building/starting from , not bolting on later ?

like the extended engishNews and frenchNews classes , now pointless unless there is something we have not been told about that would require seperate or extended classes for handling french news any differently from other languages.

I am not sure how far Avido wants to go with the translation thing , if its only a few languages this `all in one db` approach may work but if the list grows past a few , then seperate tables, possibly even DB's for each language would be preferable , this would of course in itself solve the issues above , you then only need change the tablename (hopefully dynamically rather than by duplication)

[Avido]

Ok, some explanation first

the development of the site has a history of about 2 years(I've use most of the time creating sites for customers).
It's untill recently that I've started to try thinking in an object oriented way.

So, you can see on the following image how a newsitem is displayed.(is in dutch because the translations aren't ready yet)


You have an image, a title and a newsfact. The dates are used to create the navigation(31 days, 12 months and the available years).
When i've created this a couple of months ago, i used functions to get the desired result.

Now, if you know the above fact, what should be better? Leave it like this(because it works ) and use an array to store the data in, or place this functionality(to get the navigation) inside the class(or a differen class)?

Btw, thx for the replies allready made.