B04f7f475867f6b47a59b49dfabc0daf

I'm new to the concept of getting and setter methods and I'm wondering if the need to use them applies to my code below. Any comments on the security of how i'm doing things below would be appreciated.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
<?php 

class ccForm
{
	var $action;										// Action (entsupportcase, general question, etc)
	var $emailaddy;										// E-mail Address
	var $formtype;										// Type of form (contact, new support case, etc)
	var $act_id; 										// Account ID
	var $uid;										// User ID
	var $qbuser = 'xxx';						                        // QAPI Username
	var $qbpass = 'xxx';							                // API Password
	var $rid;										// Record ID
	var $caseid;										// Case ID
	var $rsvd;										// Case Resolved (0,1)
	var $case_rep;										// First Name of Case Rep
	var $wrapper = '../includes/api.php';				                        // PHP Wrapper Location	
	
	
	public function __construct()
	{
		$this->act_id = $_GET['aui'];
		$this->uid = $_GET['dui'];
		$this->emailaddy = $_GET['liame'];
		$this->action = $_GET['aid'];
		$this->formtype = $_POST['formtype'];
		$this->caseid = $_GET['caseid'];
		$this->rsvd = $_GET['rsvd'];	
            // .....
        }
?>

Refactorings

No refactoring yet !

B8d457d2c39911ea4c74ba7d66b9c3f7

Marco Valtas

August 8, 2008, August 08, 2008 17:31, permalink

1 rating. Login to rate!

The concept of a getter/setter is just to avoid access to the attributes of your class directly. So you provide methods for access instead of exposing the inners of your class, a example is:

The class Foo has a attribute *age*, if you expose the attribute anyone could just set <age> directly (as your code above), worst, they could set to a invalid age like -10.

Another problem with your code is that you are mixing $_GET[] with the construction of your class, this is not the right way to do it. A simple new Foo() will have a lot of side effects and your class isn´t portable nor secure. Anyone could pass any value to your class just teaking the URL.

If you have a class and want construct it, the parsing of URL paremeters should live outside the class (or a class exclusive for this task), access $_GET and $_POST inside the attendant script (the one invoked by the server) instead of mixing your class with handling the server request. To be more clear, if you want reuse this class in a non web environment you can´t because you mixed the class with the environment where it resides.

In the example bellow your handling script is the script which is the target for a URL (http://server/your_script.php), there you take care of any evil parameter, the class have a validation but in this case is more like a integrity check. In Java this happens more naturally because Java is strong typed, but in PHP you have to check the types by yourself. PHP can use type hinting but just for array() or a class.

The points are, getters/setters are for your class integrity and valid state, do not receive URL parameters inside the construction or anywhere inside your class unless you designed the class just to do the task of receiving parameters and etc. Usually in PHP you should double check the parameters received.

Hope this helps.

Foo

1
2
3
4
5
6
7
8
9
10
11
12
13
14
<?php

class Foo {
 
  private $age;

  public function set_age($p_age) {
     if($p_age >= 0) $this->age = $p_age;
  }

}

?>

handling

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
<?php
// simple validation...
if(IsSet($_GET['age'])) {

   // this should be a integer value, forcing type coercion.
   $req_age = intval($_GET['age']);
   
    // after a type coercion, the parameter is still the same size?
    if(strlen($req_age) === strlen($_GET['age'])) {
       $foo_ojb = new Foo();
       $foo_ojb->set_age($req_age); // the class should accept only a valid age.
    }

}
?>
B04f7f475867f6b47a59b49dfabc0daf

goodespeler.myopenid.com

August 12, 2008, August 12, 2008 17:58, permalink

1 rating. Login to rate!

Should you create a getter/setter function for every single var you send to the class?

05fc1aa82b7da22c15e4527f0ee67dac

Josh

August 13, 2008, August 13, 2008 17:36, permalink

2 ratings. Login to rate!

Just an example of how you'd use PHP5's magic methods __set() and __get() to do the same sort of thing, plus a little data checking.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
<?php

	class Foo
	{
		//some generalized resources
		private $bar = null;
		private $baz = null;
		
		//setting up the __set() to assign variables
		//to their proper attributes, and do some
		//basic sanitizing.
		public function __set($attribute, $value)
		{
			//run some checking on if the called attribute is even in the object
			if(!is_resource($attribute)
				throw new Exception('Invalud resource called');
				
			if(is_numeric($value))
				$value = $value;
			elseif(is_null($value))
				$value = 'NULL';
			else
				$value = "'$value'";
			$this->$attribute = $value;
		}
		
		//setting up __get() to return a specified
		//attribute's variable
		public function __get($attribute)
		{
			return $this->$attribute;
		}
	}
	
	//example of how to use it:
	$foo = new Foo;
	
	//use the __set() method to assign the variable
	$foo->bar = 'test';
	
	//use the __get() method to call the varlaible back
	print_r($foo->bar);

?>
6dc0e9a07bcff97ac9b111f36e12f1f6

Ishkur

August 14, 2008, August 14, 2008 14:54, permalink

No rating. Login to rate!

Note: the above refactoring doesnt seem to really work, but it does outline an *example* of how to use PHP5's magic methods __set() and __get().

A623216a5d2384489e012478c555d167

Juha Hollanti

August 15, 2008, August 15, 2008 09:47, permalink

No rating. Login to rate!

"Should you create a getter/setter function for every single var you send to the class?"

I guess that's the general idea. Call me sloppy if you will but sometimes i tend to go around setters and getters, simply because it's simpler. Don't get me wrong though, i wouldn't want to encourage that kind of behaviour 'cause in the end it's a time bomb.

But i guess that if you find yourself in a situation where you have loads of variables in a class that you need to be able to set and get from the outside then it might mean that the class design isn't very good, or that it should be divided into a set of smaller classes.

In my very humble opinion a perfectly valid way to set a large amount of variables is through the constructor method when initializing new instances. But that won't help you if you want to get a hold of them at a later point.

035687df00d162cec025302373ebc076

chovy.myopenid.com

October 8, 2008, October 08, 2008 18:41, permalink

No rating. Login to rate!

I tend to do special handling when I need special handling. Like data validation being different depending on the variable in question.

$foo->email_addr = 'http://www.';

You would want different validation there then say:

$foo->web_addr = 'foo@bar.com';

276d28076b0fbe3facf0e0f481f06b69

vain

October 9, 2008, October 09, 2008 17:05, permalink

No rating. Login to rate!

Some thoughts on this:
1. Magic methods are always slower than direct getter/setter methods.
2. When you use magics, then tend to use full power like ArrayAccess.
3. If you have to write a lot of getters/setters, do it!
4. Prefer static methods and variables, as they are faster.

static getter / setter

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
<?php
class DataObject
{
private static $name;

static public function getName()
{
  return self::$name;
}
 

static public function setName($name)
{
  self::$name = $name;
}
   
}

# Usage:
$name = 'John Vain';
DataObject::setName($name);
echo DataObject::getName();
?>

SPL: array access

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
<?
class DataObject extends ArrayAccess
{
    /**
     * Implementation of SPL ArrayAccess
     */

    public function offsetGet($offset)
    {
        return $this->__get($offset); # calls magical __get or direct method
    }

    public function offsetSet($offset, $value)
    {
        $this->__set($offset, $value); # calls magical __set or direct method
    }
}
# The class DataObject behaves now like an array and it's data is accessible like that.
?>

Your refactoring





Format Copy from initial code

or Cancel