Return to Answer

Commonmark migration

edited Jun 10, 2020 at 13:24

Its been a long while since I have seen your code for review. I remember when you were starting out and producing horrible code. It is very nice to see the massive improvement that you have made! This code looks quite reasonable.

##The Good

The Good

Dependency Injection. You said that you didn't do it, but you have! You passed each dependency to the constructor here:
```
 $model = new UserNew( new IUniversals() , new IDatabase(), new IText(), new IMessage() );
```
That is all there is to dependency injection, you have done it nice and simply.
Encapsulation/Interface Hiding:

You have hidden all of the properties and given a simple interface to the user of the class with a limited number of public methods.

Keep doing those good things!

The Bad

invoke does not have much meaning and is similarly named to the magic method __invoke.

Its hard to tell what invoking a user will do. It looks like it is creating a user. create might be a more meaningful name.

The Future

Things can always be done better, but endless improvement doesn't get things finished. I would recommend reading about the Single Responsibility Principle and Data Mappers. This could help simplify and increase the re-usability and maintainability of this code even further. The data mapper could also remove your dependence on a database as your persistent storage (although a database is a very good place to store things).

You have definitely come a long way, nice work.

##The Good

Dependency Injection. You said that you didn't do it, but you have! You passed each dependency to the constructor here:
```
 $model = new UserNew( new IUniversals() , new IDatabase(), new IText(), new IMessage() );
```
That is all there is to dependency injection, you have done it nice and simply.
Encapsulation/Interface Hiding:

You have hidden all of the properties and given a simple interface to the user of the class with a limited number of public methods.

Keep doing those good things!

The Bad

invoke does not have much meaning and is similarly named to the magic method __invoke.

Its hard to tell what invoking a user will do. It looks like it is creating a user. create might be a more meaningful name.

The Future

You have definitely come a long way, nice work.

The Good

Dependency Injection. You said that you didn't do it, but you have! You passed each dependency to the constructor here:
```
 $model = new UserNew( new IUniversals() , new IDatabase(), new IText(), new IMessage() );
```
That is all there is to dependency injection, you have done it nice and simply.
Encapsulation/Interface Hiding:

You have hidden all of the properties and given a simple interface to the user of the class with a limited number of public methods.

Keep doing those good things!

The Bad

invoke does not have much meaning and is similarly named to the magic method __invoke.

Its hard to tell what invoking a user will do. It looks like it is creating a user. create might be a more meaningful name.

The Future

You have definitely come a long way, nice work.

Source Link

answered Jun 18, 2012 at 5:02

Paul

answered Jun 18, 2012 at 5:02

Paul

##The Good

Dependency Injection. You said that you didn't do it, but you have! You passed each dependency to the constructor here:
```
 $model = new UserNew( new IUniversals() , new IDatabase(), new IText(), new IMessage() );
```
That is all there is to dependency injection, you have done it nice and simply.
Encapsulation/Interface Hiding:

You have hidden all of the properties and given a simple interface to the user of the class with a limited number of public methods.

Keep doing those good things!

The Bad

invoke does not have much meaning and is similarly named to the magic method __invoke.

Its hard to tell what invoking a user will do. It looks like it is creating a user. create might be a more meaningful name.

The Future

You have definitely come a long way, nice work.

lang-php