Address class in PHP 7.4 with types

Question

About this exercise

I found out there is a form of strict types in PHP 7.4, so I freshened up my PHP coding with this simple Address class. I did not use PHP since forever. So, any and all answers and comments highly appreciated. (This question is an update on its deleted version; _{only users with reputation above 10k threshold can see deleted questions})

I still do not know exactly how the exceptions are to be used. If I made some sort of error in this area, please excuse me, but this code is working on my webserver without much testing.

Additional Note (EDIT): This class is culturally specific to the Czech Republic, where, just to clarify:

• Street and City have a minimum of 2 characters.

• All of the following must be integers: House number / Orientation number, and the Zip code. There can be no letters to the House number for example, nor in the other two.

---

Code

<?php
// independent class
declare(strict_types=1);
class Address
{
 private string $street = "";
 private int $houseNumber = 0;
 private int $orientationNumber = 0;
 private string $city = "";
 private int $zip = 0;
 public function setStreet(string $newStreet): void
 {
 if (strlen($newStreet) >= 2)
 {
 $this->street = $newStreet;
 }
 else
 {
 throw new Exception("Street needs to be of length 2 or more.");
 }
 }
 public function setHouseNumber(int $newHouseNumber): void
 {
 if (is_int($newHouseNumber) && $newHouseNumber > 0)
 {
 $this->houseNumber = $newHouseNumber;
 }
 else
 {
 throw new Exception('House number needs to have a value greater than 0.');
 }
 }
 public function setOrientationNumber(int $newOrientationNumber): void
 {
 if (is_int($newOrientationNumber) && $newOrientationNumber >= 0)
 {
 $this->orientationNumber = $newOrientationNumber;
 }
 else
 {
 throw new Exception("Orientation number needs to have a value greater than 0; Can be equal to 0, in which case it won't be visible in full address.");
 }
 }
 public function setCity(string $newCity): void
 {
 if (strlen($newCity) >= 2)
 {
 $this->city = $newCity;
 }
 else
 {
 throw new Exception("City needs to be of length 2 or more.");
 }
 }
 public function setZip(int $newZip): void
 {
 if (is_integer($newZip) && $newZip >= 10000 && $newZip <= 99999 )
 {
 $this->zip = $newZip;
 }
 else
 {
 throw new Exception("Czech postal code must be integer in hypothetical range <10000;99999>.");
 }
 }
 public function __construct(
 string $initStreet,
 int $initHouseNumber,
 int $initOrientationNumber,
 string $initCity,
 int $initZip)
 {
 $this->setStreet($initStreet);
 $this->setHouseNumber($initHouseNumber);
 $this->setOrientationNumber($initOrientationNumber);
 $this->setCity($initCity);
 $this->setZip($initZip);
 }
 public function getStreet(): string
 {
 return $this->street;
 }
 public function getHouseNumber(): int
 {
 return $this->houseNumber;
 }
 public function getOrientationNumber(): int
 {
 return $this->orientationNumber;
 }
 public function getCity(): string
 {
 return $this->city;
 }
 public function getZip(): int
 {
 return $this->zip;
 }
 public function getFullAddress(): string
 {
 $fullAddress = $this->street . " " . $this->houseNumber;
 if ($this->orientationNumber !== 0)
 {
 $fullAddress .= "/" . $this->orientationNumber;
 }
 $fullAddress .= ", " . $this->zip . " " . $this->city;
 return $fullAddress;
 }
}

• KIKO Software: I would use strings everywhere for addresses. For example, ZIP codes can start with a zero. I don't think there's much to review here, unless you actually plan to use this class for something.
• mickmackusa: If you are coercing int with int $newHouseNumber, then the is_int($newHouseNumber) check is useless.
• Vlastimil Burián: @KIKOSoftware Deployed already on my company's web, but not of much use since I store only my address there. Anyway, if there is a space to improve, then improve it.
• Vlastimil Burián: @mickmackusa I see. So, basically, I don't need to check it. Ok.
• Chris Haas: Is there a use-case for the setters beyond the constructor? If not could they be made private or the logic moved into the constructor? I don't want to go down the rabbit hole about immutable objects, but unless you have a need to create an object with all required fields, and then allow for changing those, you could reduce your surface area. I also wanted to point out that PHP 8 is around the corner which you might want to look into, specifically constructor property promotion. I'm not saying it is good or bad, just that it exists.

Answer 1

I saw your post yesterday before it was deleted. I didn’t have much to suggest but after seeing this code there are a few things I would change.

The suggestions by YourCommonSense are great:

• Specific exception sub-classes (which can be custom) can help determine how edge cases should be handled by code that calls your code.
• With strict typing enabled, a non-integer passed to a function that expects an integer would lead to a TypeError¹ so the calls to is_int() in the methods that expect integers for parameters is superfluous.

Suggestions

Constructor placement

Many developers writing OOP code will place the constructor before all other methods in the class definitions, like the code in your previous post contained. This is not required and many IDEs would have a quick way to find it but it helps you (or a teammate) find it quicker than hunting through the source.

Default values for member/instance variables

The default values are great, though given that the constructor requires all to be passed (which then get passed to the setters) it makes the default values superfluous.

Docblocks

PSR-5 is in draft status currently but is commonly followed amongst PHP developers. It recommends adding docblocks above structural elements like classes, methods, properties, etc. Many popular IDEs will index the docblocks and use them to suggest parameter names when using code that has been documented. At least do it for the methods like the constructor, in case you forget which order the parameters are in.

Actually given the type declarations for the parameters and the return values some argue those aspects of the docblocks aren't as critical. The main things to focus on are comments that may not be obvious after reading a method name, as well as any possible exceptions that may be thrown by calling the method.

Avoiding else when possible

In this presentation about cleaning up code Rafael Dohms talks about returning early, limiting the indentation level to one per method and avoiding the else keyword. (see the slides here).

Most of the setter methods have a validation check with an else block. If the logic was flipped - i.e. when validation fails then throw the exception, then the else keyword could be eliminated. This will keep indentation levels at a minimum. For small methods like these it likely won’t make a big difference but in larger methods it would be significant.

Let’s look at one setter:

public function setStreet(string $newStreet): void
{
 if (strlen($newStreet) >= 2)
 {
 $this->street = $newStreet;
 }
 else
 {
 throw new Exception("Street needs to be of length 2 or more.");
 }
 }

Moving the throw statement up will allow the method to be shortened and indentation levels decreased:

public function setStreet(string $newStreet): void
{
 if (strlen($newStreet) < 2)
 {
 throw new Exception("Street needs to be of length 2 or more.");
 }
 $this->street = $newStreet;
}

The else is not needed, since "code following the [throw] statement will not be executed, and PHP will attempt to find the first matching catch block"²

• Vlastimil Burián: Did you not mention those two very good suggestions from the other answer intentionally. They are definitely a thing to implement for me. Anyway, thanks for the further hints! 🙏
• Sᴀᴍ Onᴇᴌᴀ: I agree with the suggestions in the answer by YourCommonSense, and I often state that though I neglected to do so initially. After reading this meta perhaps I should make it a habit.

Answer 2

I was able to come up with two suggestions so far

Using a generic exception is definitely not the way to go. Ask yourself a question, what your code is supposed to do when such an exception is thrown? displaying it to a user? but what if an exception was not a validation error at all but some serious system error, such as "class not found" for example? You don't want to show such errors to a site user.

At least make it InvalidArgumentException, which is better suited for this kind of error.

Better yet, create a custom InvalidAddressArgumentException and throw this one, so you will be able to catch only Address validation errors and leave all other errors alone.

And a petty issue, using is_int() for an argument that is already typehinted makes no sense. It will never return false.

• hjpotter92: Additionally, since you do have getVar() system in place, use those methods in the getFullAddress as well @LinuxSecurityFreak
• Your Common Sense: @LinuxSecurityFreak that's actually a different question: why did you make all those getters? For which purpose? Probably an answer to this will lead you to the answer to yours
• mickmackusa: "petty" ...I'm heartbroken. :)
• Vlastimil Burián: The second answer seems more in-depth. However, thank you for your time and those two good suggestions! 🙏