# Finding the right structure for my application

I have three different types of XML files (structure) which I would like to import into a database.

Every hour a cronjob calls my import.php with a specific XML file type as a parameter.

The import script includes a bootstrap.php (AutoLoader, Setup Doctrine DB Connection) and puts all files matching the name convention of this type in an array and calls a static method for this class.

Simplified code (import.php):

<?php
require_once("bootstrap.php");

function getFiles($pattern) {$fileList = NULL;
foreach( glob('xml/*'.$pattern.'.xml') as$filename )
{
$fileList[] =$filename;
}

return $fileList; } switch($xmlType)
{
case "TypeA":
TypeA::importFeed(getFiles($xmlType)); break; case "TypeB": TypeB::importFeed(getFiles($xmlType));
break;

case "TypeC":
TypeC::importFeed(getFiles($xmlType)); break; }  Currently, the classes (TypeA, TypeB, TypeC) contain no other method except importFeed. Inside importFeed, only 2 things are equal in each class: 1. Getting the entityManager (Doctrine) by calling $em = app::getEm();
2. Convert XML into SimpleXMLElement using my parser class $feedObj = Parser::xmlToObject($xmlFile);

Everything else in importFeed differs from class to class because of the different XML types. Only a few tasks repeating e.g. check if an entity already exists in the database.

My first thought was creating a Helper class for this and some other repeating tasks I also need outside the import-script in this application.

But I'm not sure if this helper class should be a normal, "static" or "singleton" class.

The helper-methods doesn't need a specific object instance so static-methods would a good choice. But in some methods I need access to the DB and I do not wont to call app::getEm(); in every method because I don't think this is a good/clean solution.

At this point I challenged my current application design and read a lot of articles about this. The point is the application is not large and it should not be so using a Framework like Symfony is completely exaggerated - nonetheless I want a good solution.

All three classes (TypeA/B/C) have the same function that looks like this:

public static function importFeed($xmlFiles) {$em = \App::getEm();
foreach($xmlFiles as$xmlFile)
{
try
{
$feedObj = Parser::xmlToObject($xmlFile);
...import xml data to database - differns ...
}
catch(\Exception $e) { .... } } }  Only the "..." area differs a lot because of the different XML structure but this part isn't really important for the question. In short: A Doctrine entity is created and many setAttributeX methods are called and in the end $em->flush().

My feeling tells me that it is not right to have three classes with only static methods to import XML files, but I can't find a better solution.

• create an abstract class that is then extended by TypA, TypeB,... Jul 4, 2013 at 18:45
• There are no methods that I need in all classes (TypeA, TypeB, TypeC) therefore I could better use a Interface to ensure all needed function of a "Import"-Class are implemented. But I think the major problem is the "3 Classes with only static methods"-Solution. Jul 5, 2013 at 12:44
• I agree with @Pinoniq (called him out since you didn't) If you make a abstract marker class that is extended by typeA, B,C you can make a few methods that all 3 share in the marker class. And you can make a GetXMLType (name??) that returns the correct instance of the Parser you need. That would clean up your code a bunch. Any common methods can be made in the Abstract class so you don't repeat your self. Jul 5, 2013 at 13:23
• @Robert Snyder Thanks for your feedback. The "problem" is that no methods are shared between the three classes. It's too individually. The only repeating tasks I would like to have in a Helper-Class because I also need them outside the import functionality. I thought my approach using three classes with only static methods was completly wrong but If i understand you correctly it's nothing wrong with it? Jul 5, 2013 at 13:54
• @Manuel Personally I try to avoid static methods since they make me right bad code. But that's because I tend to get very lazy sometimes. I also alway use dependency injection because that makes unit tests a lot easier. Just my 5 cents Jul 5, 2013 at 14:00

I would go somewhere in the lines of the following code:

abstract class ObjectImporter {

protected $em; public function __construct($em) {
$this->em =$em;
}

abstract public function import($object); } class TypeAImporter extends ObjectParser { public function __construct($em) {
parent::__construct($em); } public function import($object) {
}
}

class XmlImporter {

private $importer; public function __construct($importer) {
$this->importer =$importer;
}

public function import($files) { foreach ($files as $file ) {$obj = Parser::xmlToObject($file);$this->importer->import($obj); } } } switch ($xmlType ) {

case 'TypeA':
$importer = new XmlImporter(new TypeAImporter(\App::getEm())); break; }$importer->import(\$files);


Some notes: the code is not ideal (names aren't that good imo) and I don't like the Parser::xmlToObject method. The Parser should be injected into the XmlImporter class aswel. This way its easier tested and you can also very easily change the xml Parser.

I hope the code is of any use.

• Thank you for this example. Looks much tidier as mine. Although I actually do not need a individually class instance for this task because the processing is object independently but it seems more a question of what harms a instance instead of whats the benefit. Or I'm wrong? Jul 5, 2013 at 14:33
• It's more a question of how easy do you want to be able to run Tests on your code. At my work everything gets unit tested. even if I remove some white-spaces. That is why I use dependency injection instead of using static classes or using the 'new' operator inside a class. More info here: stackoverflow.com/questions/1580641/… for instance. So the benefit is the ability to write better an smaller tests ;) Jul 5, 2013 at 15:29