Page 1 of 1

is this a good way of writing OOP

Posted: Fri May 12, 2006 11:19 am
by AshrakTheWhite
Just what the topic says:) opinnions please!
Main:

Code: Select all

<?php
require_once 'core.php';
require_once 'printer.php';

$print = new Printer;

echo $print->main_table;
?>
Core:

Code: Select all

<?php
session_start();
?>

Code: Select all

<?php
require_once 'core.php';
class Make_vars
{
    public $ostja_nimi;
    public $ostja_aadress;
    public $arve_nr;
    public $arve_kuupaev;
    public $maksetahtaeg;
    public $kirjeldus_nimi;
    public $kogus_nr;
    public $hind_nr;
    public $summa_nr;
    public $kaibemaks_nr;
    public $markused_nimi;
    
    public function __construct()
    {
        $this->ostja_nimi = $_POST['ostja_nimi'];
        $this->ostja_aadress = $_POST['ostja_aadress'];
        $this->arve_nr = $_POST['arve_nr'];
        $this->kirjeldus_nimi = $_POST['kirjeldus_nimi'];
        $this->kogus_nr = $_POST['kogus_nr'];
        $this->hind_nr = $_POST['hind_nr'];
        $this->markused_nimi = $_POST['markused_nimi'];
        $this->uhik_nimi = $_POST['uhik_nimi'];
    }

}

?>

Code: Select all

<?php
require_once 'make_vars';

class Math
{   
    private make_vars;

    public function __construct()
    {
        $this->make_vars = new Make_vars;
        $this->make_vars->kaibemaks_nr = 0.18;
    }
    

    public function arve_kuupaev()
    {   
        $this->arve_kuupaev = mktime(0, 0, 0, date("m"), date("d"),  date("Y"));
        return $this->arve_kuupaev;
    }

    public function maksetahtaeg()
    {
        $this->arve_kuupaev = $this->arve_kuupaev = mktime(0, 0, 0, date("m"), date("d") + 7,  date("Y"));
        return $this->maksetahtaeg;
    }
    
    public function getItemSum()
    {
        $out = $this->make_vars->hind_nr * $this->make_vars->kogus_nr;
        return $out;
    }

    public function getTotalSum()
    {
        $total = new array();
        $total = $this->getItemSum();
    }

}

?>

Posted: Fri May 12, 2006 11:20 am
by AshrakTheWhite

Code: Select all

<?php
require_once 'Core.php';
require_once 'Make_vars.php';
require_once 'math.php';

class Printer
{
    private $do_math;
    private $make_vars;
    public $main_table;
    public $input_table;

    public function __construct()
    {
        $this->make_vars = new Make_vars;
        $this->do_math = new Math;
        $this->main_table = $this->printer();
    }

    public function printer()
    {
        if(isset($this->make_vars->ostja_nimi, $this->make_vars->arve_nr, $this->make_vars->kirjeldus_nimi, $this->make_vars->kogus_nr, $this->make_vars->hind_nr, $this->make_vars->allahindlus_nr))
        {
            $out = '
                <!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
                <html xmlns="http://www.w3.org/1999/xhtml">
                <head>
                <meta http-equiv="Content-Type" content="text/html; charset=iso-8859-1" />
                <title>Arve</title>
                <link href="primary.css" rel="stylesheet" type="text/css" />
                </head>

                <body>
                <table width="800" border="1" cellpadding="0" cellspacing="0">
                   <tr>
                      <td width="308" height="133" valign="top"><p class="bold">Arve Nr. '. $this->make_vars->arve_nr .'</p>
                         <p>Arve kuupäev: '. $this->do_math->arve_kuupaev .'<br />
                            Maksetähtaeg: '. $this->do_math->maksetaehtaeg .'</p>
                      <p>&nbsp;</p>
                      <span class="bold">Ostja:</span></td>
                      <td width="292" rowspan="2" valign="top"><p class="allikiri"><br />
                         <br />
                         <span class="firma">NOTOS INVEST O&Uuml;
                         <br />
                         Reg. kood: 11033401<br />
                         S&auml;rgava allee 6 12013 Tallinn<br />
                telefon: 6420567<br />
                info@mugav.ee<br />
                             a/a: 10220036974019</span></p>
                      </td>
                   </tr>
                   
                   
                   <tr>
                      <td height="92" valign="top">'. $this->make_vars->ostja_nimi .'<br />
                      '. $this->make_vars->ostja_aadress .'
                      </td>
                   </tr>
                </table>
                <table width="800" border="1" cellpadding="0" cellspacing="0">
                   <tr>
                      <td width="400" height="20" valign="top">Nimetus</td>
                        <td width="100" valign="top">M&auml;rkused</td>
                        <td width="100" valign="top">Kogus</td>
                        <td width="100" valign="top">&Uuml;hik</td>
                        <td width="100" valign="top">Hind</td>
                        <td width="98" valign="top">Summa</td>
                   </tr>
                   '. .'
                   <tr>
                      <td width="400" height="20" valign="top">'. $this->make_vars->kirjeldus_nimi .'</td>
                        <td width="100" valign="top">'. $this->make_vars->markused_nimi .'</td>
                        <td width="100" valign="top">'. $this->make_vars->kogus_nr .'</td>
                        <td width="100" valign="top">'. $this->make_vars->uhik_nimi .'</td>
                        <td width="100" valign="top">'. $this->make_vars->hind_nr .'</td>
                        <td width="98" valign="top">'. $this->do_math->getItemSum .'</td>
                   </tr>
                </table>
                <table width="800" border="1" cellpadding="0" cellspacing="0">
                   <tr>
                      <td width="667" height="60" valign="top" class="bold2">Maksumus k&auml;ibemaksuta: <br />
                   K&auml;ibemaks 18%: <br />
                   Kokku:&nbsp;</td>
                   <td width="127" valign="top">var k&auml;ibemaks<br />
                      var 18%<br />
                      varkokku</td>
                   </tr>
                </table>
                <table width="800" border="1" cellpadding="0" cellspacing="0">
                   <tr>
                      <td width="800" height="206" valign="top"><p>Tasumisele kuulub: </p>
                         <p>&nbsp;</p>
                         <p>Pretensioonid t&ouml;&ouml; teostamise kohta esitada 24h jooksul peale t&ouml;&ouml; vastuv&otilde;tmist. </p>
                      <p>&nbsp;</p>         
                      <p class="allikiri">V&auml;ljastas: ................................... &nbsp;&nbsp;&nbsp;T&ouml;&ouml; ja arve k&auml;tte saadud: ..................................</p></td>
                   </tr>
                </table>
                </body>
                </html>
            ';
            return $out;
        } else
            {
                $out = '
                    <HTML>
                    <title>
                    Input the values Please!
                    </title>
                    <body>
                        <form action="Main.php" method="post">
                            Ostja Nimi:<input type=text name="ostja_nimi">
                            Ostja Aadress:<input type=text name="ostja_aadress">
                            arve number:<input type=text name="arve_nr">
                            kirjeldus:<input type=text name="kirjeldus_nimi">
                            kogus:<input type=text name="kogus_nr">
                            hind:<input type=text name="hind_nr">
                            Märkused:<input type=text name="markused_nimi">
                            Ühik:<input type=text name="uhik_nimi">
                            <input type="submit" name="submit" value="send">
                        </form>
                    </body>
                    </HTML>
                ';
        
                return $out;
            }    
    }
}
?>

Posted: Fri May 12, 2006 11:27 am
by alex.barylski
2 comments:

1) Seeing how your name (alias/handle) implies racial cleansing...in particularly the race I happen to belong too :P

I'm not going to offer you much advice...

2) I don't really care actually, i'm just bugging, although I question your motives...

Your storing HTML inside an OOP class and your asking if it's proper design?

No...thats bad practice...and I'm not even going to get into why as I've argued my point a 100 times over...

Cheers :)

Posted: Fri May 12, 2006 11:32 am
by shiznatix
i would definatly seperate your HTML from your class if possible.

also I would not do this:

Code: Select all

public function __construct()
    {
        $this->ostja_nimi = $_POST['ostja_nimi'];
        $this->ostja_aadress = $_POST['ostja_aadress'];
        $this->arve_nr = $_POST['arve_nr'];
        $this->kirjeldus_nimi = $_POST['kirjeldus_nimi'];
        $this->kogus_nr = $_POST['kogus_nr'];
        $this->hind_nr = $_POST['hind_nr'];
        $this->markused_nimi = $_POST['markused_nimi'];
        $this->uhik_nimi = $_POST['uhik_nimi'];
    }
instead I would do something like this:

Code: Select all

public function __construct($nimi, $aadress, $nr, $client_name, $misOn_nr, $price_nr, $marked_name, $kuidas_name)
{
        $this->ostja_nimi = $nimi;
        $this->ostja_aadress = $aadress;
        $this->arve_nr = $nr;
        $this->kirjeldus_nimi = $client_name;
        $this->kogus_nr = $misOn_nr;
        $this->hind_nr = $price_nr;
        $this->markused_nimi = $marked_name;
        $this->uhik_nimi = $kuidas_name;
}
then when you call it be like:

Code: Select all

$classInstance = new Make_vars($_POST[].......);//fill in yer post stuff there
also, it would be easiest for other coders that might have to look at your stuff if you stuck to 1 language for your variable/class/function names instead of switching at random. English would be the prefered because not tooo many people outside of estonia know estonian ;)

Posted: Fri May 12, 2006 11:32 am
by AshrakTheWhite
Hockey wrote:2 comments:

1) Seeing how your name (alias/handle) implies racial cleansing...in particularly the race I happen to belong too :P

I'm not going to offer you much advice...
Ashrak - Gou'Uld assasin from the seriest stargate sg-1
TheWhite - Reference to Gandalf The White or Saruman the white from Lotr trilogy
2) I don't really care actually, i'm just bugging, although I question your motives...

Your storing HTML inside an OOP class and your asking if it's proper design?

No...thats bad practice...and I'm not even going to get into why as I've argued my point a 100 times over...

Cheers :)

Mind giving a link to wherey ouv argumented it? :)


EDIT: Also the argument names are in estonian because i have no freaking idea on how they translate to english :p

Posted: Fri May 12, 2006 12:37 pm
by Christopher
There are a couple of things. First is your Make_vars class is usually refered to as something like HTTP_Request because that's what it is. Since you are using PHP5, something like this might be better:

Code: Select all

class HTTP_Request {

    public function __get($key) {
        return isset($_POST[$key]) ? $_POST[$key] : null;
    }

}
And you Printer class is usually called something like View. Also it is usually better (for reuse, testing, etc.) to not have internal dependencies on instansiating classes, so something like this might be better:

Code: Select all

class View
{
    private $do_math;
    private $make_vars;
    public $main_table;
    public $input_table;

    public function __construct($request, $model)
    {
        $this->make_vars = $request;
        $this->do_math = $model;
        $this->main_table = $this->printer();
    }
Created like this:

Code: Select all

$view = new View(new Request, new Math);
Notice how things are falling out naturally into the MVC pattern.

Re: is this a good way of writing OOP

Posted: Fri May 12, 2006 1:08 pm
by timvw
AshrakTheWhite wrote:Just what the topic says:) opinnions please!
Please repeat your question in the body itself.. Anyway, before i can judge on your code, how could you define 'good way of writing OOP' ?

Re: is this a good way of writing OOP

Posted: Fri May 12, 2006 1:15 pm
by Christopher
timvw wrote:Anyway, before i can judge on your code, how could you define 'good way of writing OOP' ?
I think the question, "Is this a good way of writing OOP?" implied that the OP wasn't sure what "good OOP" is. I'm not sure that I know, but luckily they just asked for opinions -- not facts.

Posted: Fri May 12, 2006 3:00 pm
by AshrakTheWhite
thats my first try of OOP and my 2nd week of coding, so i need to develop standards so people would understand what the heck is going on ;)

Posted: Fri May 12, 2006 3:02 pm
by timvw
Here's mine opinion: Did the code meet the customer requirements? If yes, then it's good (but there is room for improvement). If no, then it's not good.

Eg: What is the purpose of the following?? It doesn't return anything and it doesn't change anything... So i wonder ;)

Code: Select all

public function getTotalSum()
    {
        $total = new array();
        $total = $this->getItemSum();
    }

Posted: Fri May 12, 2006 3:47 pm
by Christopher
AshrakTheWhite wrote:thats my first try of OOP and my 2nd week of coding, so i need to develop standards so people would understand what the heck is going on ;)
I think it is a very good first try. Maybe getting a book or searching the net about some programming basics of dependencies, layering, modularization, etc. would be a good thing to do. I think if you read up on pattern, specifically on MVC in PHP, and just let it soak in knowing that you won't understand much at first would be good as well.

Posted: Fri May 12, 2006 4:52 pm
by AshrakTheWhite
timvw wrote:Here's mine opinion: Did the code meet the customer requirements? If yes, then it's good (but there is room for improvement). If no, then it's not good.

Eg: What is the purpose of the following?? It doesn't return anything and it doesn't change anything... So i wonder ;)

Code: Select all

public function getTotalSum()
    {
        $total = new array();
        $total = $this->getItemSum();
    }

unfinished function :p

its supposed to add together all the sums from the previous function, yet to figure out how to do that :(