Some observations and suggestions:
[a] You are passing a ModModel
object to ModUploadModel
to validate the mod name before uploading. You shouldn't even try to call ModUploadModel::upload()
if a mod with the provided name already exists. So you should follow steps similar to this:
class ModUploadController extends Mvc\Controller {
public function uploadMod(): void {
$modUploadModel = new ModUploadModel()
$modModel = new ModModel();
$modName = 'beatiful-mod';
try {
if ($modModel->doesModNameExists($modName)) {
throw new \ModNameExistsException('A mod with the name "' . $modName . '" already exists');
}
$modUploadModel->upload($modName);
} catch (\ModNameExistsException $exception){
// ...Present the exception message to the user. Use $exception->getMessage() to get it...
}
}
}
[b] Creating objects inside a class is a bad idea (like in ModUploadController
). Use dependency injection instead. Read this and watch this and this. So the solution would look something like this:
class ModUploadController extends Mvc\Controller {
public function uploadMod(ModUploadModel $modUploadModel, ModModel $modModel): void {
//... Use the injected objects ($modUploadModel and $modModel ) ...
}
}
In a project, all objects that need to be injected into others can be created by a "dependency injection container". For example, PHP-DI (which I recommend), or other DI containers. So, a DI container takes care of all dependency injections of your project. For example, in your case, the two objects injected into ModUploadController::uploadMod
method would be automatically created by PHP-DI. You'd just have to write three lines of codes in the file used as the entry-point of your app, probably index.php
:
use DI\ContainerBuilder;
$containerBuilder = new ContainerBuilder();
$containerBuilder->useAutowiring(true);
$container = $containerBuilder->build();
Of course, a DI container requires configuration steps as well. But, in a couple of hours, you can understand how and where to do it.
By using a DI container, you'll be able to concentrate yourself solely on the logic of your project, not on how and where various components should be created, or similar tasks.
[c] Using static methods is a bad idea. My advise would be to get rid of all static methods that you already wrote. Watch this, read this, this and this. So the solution to the injection problem(s) that you have is the one above: the DI, perfomed by a DI container. Not at all creating static methods.
[d] You are using both components to query the database (ModModel
with doesModNameExists()
and ModUploadModel
with insertIntoDb()
). You should dedicate only one component to deal with the database.
[e] You don't need Mvc\Model
at all.
[f] You don't need Mvc\Controller
at all.
Some code:
I wrote some code, as an alternative to yours (from which I somehow "deduced" the tasks). Maybe it will help you, seeing how someone else would code. It would give you the possibility of "adding features to my mvc website without do any sort of spaghetti code". The code is very similar to the one from an answer that I wrote a short time ago. That answer also contains additional important suggestions and resources.
Important: Note that the application services, e.g. all components from Mvc/App/Service/
, should communicate ONLY with the domain model components, e.g. with the components from Mvc/Domain/Model/
(mostly interfaces), not from Mvc/Domain/Infrastructure/
. In turn, the DI container of your choice will take care of injecting the proper class implementations from Mvc/Domain/Infrastructure/
for the interfaces of Mvc/Domain/Model/
used by the application services.
Note: my code uses PHP 8.0. Good luck.
Project structure:

Mvc/App/Controller/Mod/AddMod.php:
<?php
namespace Mvc\App\Controller\Mod;
use Psr\Http\Message\{
ResponseInterface,
ServerRequestInterface,
};
use Mvc\App\Service\Mod\{
AddMod As AddModService,
Exception\ModAlreadyExists,
};
use Mvc\App\View\Mod\AddMod as AddModView;
class AddMod {
/**
* @param AddModView $addModView A view for presenting the response to the request back to the user.
* @param AddModService $addModService An application service for adding a mod to the model layer.
*/
public function __construct(
private AddModView $addModView,
private AddModService $addModService,
) {
}
/**
* Add a mod.
*
* The mod details are submitted from a form, using the HTTP method "POST".
*
* @param ServerRequestInterface $request A server request.
* @return ResponseInterface The response to the current request.
*/
public function addMod(ServerRequestInterface $request): ResponseInterface {
// Read the values submitted by the user.
$name = $request->getParsedBody()['name'];
$description = $request->getParsedBody()['description'];
// Add the mod.
try {
$mod = $this->addModService->addMod($name, $description);
$this->addModView->setMod($mod);
} catch (ModAlreadyExists $exception) {
$this->addModView->setErrorMessage(
$exception->getMessage()
);
}
// Present the results to the user.
$response = $this->addModView->addMod();
return $response;
}
}
Mvc/App/Service/Mod/Exception/ModAlreadyExists.php:
<?php
namespace Mvc\App\Service\Mod\Exception;
/**
* An exception thrown if a mod already exists.
*/
class ModAlreadyExists extends \OverflowException {
}
Mvc/App/Service/Mod/AddMod.php:
<?php
namespace Mvc\App\Service\Mod;
use Mvc\Domain\Model\Mod\{
Mod,
ModMapper,
};
use Mvc\App\Service\Mod\Exception\ModAlreadyExists;
/**
* An application service for adding a mod.
*/
class AddMod {
/**
* @param ModMapper $modMapper A data mapper for transfering mods
* to and from a persistence system.
*/
public function __construct(
private ModMapper $modMapper
) {
}
/**
* Add a mod.
*
* @param string|null $name A mod name.
* @param string|null $description A mod description.
* @return Mod The added mod.
*/
public function addMod(?string $name, ?string $description): Mod {
$mod = $this->createMod($name, $description);
return $this->storeMod($mod);
}
/**
* Create a mod.
*
* @param string|null $name A mod name.
* @param string|null $description A mod description.
* @return Mod The newly created mod.
*/
private function createMod(?string $name, ?string $description): Mod {
return new Mod($name, $description);
}
/**
* Store a mod.
*
* @param Mod $mod A mod.
* @return Mod The stored mod.
* @throws ModAlreadyExists The mod already exists.
*/
private function storeMod(Mod $mod): Mod {
if ($this->modMapper->modExists($mod)) {
throw new ModAlreadyExists(
'A mod with the name "' . $mod->getName() . '" already exists'
);
}
return $this->modMapper->saveMod($mod);
}
}
Mvc/App/View/Mod/AddMod.php:
<?php
namespace Mvc\App\View\Mod;
use Mvc\{
App\View\View,
Domain\Model\Mod\Mod,
};
use Psr\Http\Message\ResponseInterface;
/**
* A view for adding a mod.
*/
class AddMod extends View {
/** @var Mod A mod. */
private Mod $mod = null;
/**
* Add a mod.
*
* @return ResponseInterface The response to the current request.
*/
public function addMod(): ResponseInterface {
$bodyContent = $this->templateRenderer->render('@Templates/Mod/AddMod.html.twig', [
'activeNavItem' => 'AddMod',
'mod' => $this->mod,
'error' => $this->errorMessage,
]);
$response = $this->responseFactory->createResponse();
$response->getBody()->write($bodyContent);
return $response;
}
/**
* Set the mod.
*
* @param Mod $mod A mod.
* @return static
*/
public function setMod(Mod $mod): static {
$this->mod = $mod;
return $this;
}
}
Mvc/App/View/View.php:
<?php
namespace Mvc\App\View;
use Psr\Http\Message\ResponseFactoryInterface;
use SampleLib\Template\Renderer\TemplateRendererInterface;
/**
* A view.
*/
abstract class View {
/** @var string An error message */
protected string $errorMessage = '';
/**
* @param ResponseFactoryInterface $responseFactory A response factory.
* @param TemplateRendererInterface $templateRenderer A template renderer.
*/
public function __construct(
protected ResponseFactoryInterface $responseFactory,
protected TemplateRendererInterface $templateRenderer
) {
}
/**
* Set the error message.
*
* @param string $errorMessage An error message.
* @return static
*/
public function setErrorMessage(string $errorMessage): static {
$this->errorMessage = $errorMessage;
return $this;
}
}
Mvc/Domain/Infrastructure/Mod/PdoModMapper.php:
<?php
namespace Mvc\Domain\Infrastructure\Mod;
use Mvc\Domain\Model\Mod\{
Mod,
ModMapper,
};
use PDO;
/**
* A data mapper for transfering Mod entities to and from a database.
*
* This class uses a PDO instance as database connection.
*/
class PdoModMapper implements ModMapper {
/**
* @param PDO $connection Database connection.
*/
public function __construct(
private PDO $connection
) {
}
/**
* @inheritDoc
*/
public function modExists(Mod $mod): bool {
$sql = 'SELECT COUNT(*) as cnt FROM mods WHERE name = :name';
$statement = $this->connection->prepare($sql);
$statement->execute([
':name' => $mod->getName(),
]);
$data = $statement->fetch(PDO::FETCH_ASSOC);
return ($data['cnt'] > 0) ? true : false;
}
/**
* @inheritDoc
*/
public function saveMod(Mod $mod): Mod {
if (isset($mod->getId())) {
return $this->updateMod($mod);
}
return $this->insertMod($mod);
}
/**
* Update a mod.
*
* @param Mod $mod A mod.
* @return Mod The mod.
*/
private function updateMod(Mod $mod): Mod {
$sql = 'UPDATE mods
SET
name = :name,
description = :description
WHERE
id = :id';
$statement = $this->connection->prepare($sql);
$statement->execute([
':name' => $mod->getName(),
':description' => $mod->getDescription(),
]);
return $mod;
}
/**
* Insert a mod.
*
* @param Mod $mod A mod.
* @return Mod The newly inserted mod.
*/
private function insertMod(Mod $mod): Mod {
$sql = 'INSERT INTO mods (
name,
description
) VALUES (
:name,
:description
)';
$statement = $this->connection->prepare($sql);
$statement->execute([
':name' => $mod->getName(),
':description' => $mod->getDescription(),
]);
$mod->setId(
$this->connection->lastInsertId()
);
return $mod;
}
}
Mvc/Domain/Model/Mod/Mod.php:
<?php
namespace Mvc\Domain\Model\Mod;
/**
* Mod entity.
*/
class Mod {
/**
* @param string|null $name (optional) A name.
* @param string|null $description (optional) A description.
*/
public function __construct(
private ?string $name = null,
private ?string $description = null
) {
}
/**
* Get id.
*
* @return int|null
*/
public function getId(): ?int {
return $this->id;
}
/**
* Set id.
*
* @param int|null $id An id.
* @return static
*/
public function setId(?int $id): static {
$this->id = $id;
return $this;
}
/**
* Get the name.
*
* @return string|null
*/
public function getName(): ?string {
return $this->name;
}
/**
* Set the name.
*
* @param string|null $name A name.
* @return static
*/
public function setName(?string $name): static {
$this->name = $name;
return $this;
}
/**
* Get the description.
*
* @return string|null
*/
public function getDescription(): ?string {
return $this->description;
}
/**
* Set the description.
*
* @param string|null $description A description.
* @return static
*/
public function setDescription(?string $description): static {
$this->description = $description;
return $this;
}
}
Mvc/Domain/Model/Mod/ModMapper.php:
<?php
namespace Mvc\Domain\Model\Mod;
use Mvc\Domain\Model\Mod\Mod;
/**
* An interface for various data mappers used to
* transfer Mod entities to and from a persistence system.
*/
interface ModMapper {
/**
* Check if a mod exists.
*
* @param Mod $mod A mod.
* @return bool True if the mod exists, false otherwise.
*/
public function modExists(Mod $mod): bool;
/**
* Save a mod.
*
* @param Mod $mod A mod.
* @return Mod The saved mod.
*/
public function saveMod(Mod $mod): Mod;
}