u/RealWorldPHP

35 post karma · 132 comment karma · joined Sep 2018

RealWorldPHP

Okay, I see. You had already understood the rule being broken.

As far as the interface being empty... I left it empty because there was no common behavior that needed to be enforced. It is a way to tie those DTOs together and also keep the create function signature the same. If the constraint DTOs were to have a behavior, then that would go into the interface. Of course, if you have a DTO with behavior, then maybe you don't have a data transfer object anymore. And that's okay, too. Either way, I think it makes for cleaner code.

RealWorldPHP

One more thought... It could be that you want to always make sure that there is a slug and a contentType available. So another option would be to use an Abstract Class instead of the interface. And have the constraint classes be value objects instead of DTOs with validation built in. There is always another way to do things in programming. Pick your poison and make the most of it.

RealWorldPHP

I am sure others have the same answer as I do, but it is all too much to read to make sure. So if this is redundant, I apologize.

You want to know why this violates LSP. Here is the place in the documentation that is explains the rule you broke: https://www.php.net/manual/en/language.oop5.basic.php#language.oop.lsp

Your create method in UserContentCreator is overriding the create method in the ContentCreator class. In order to override a method, you must make the overriding method have signature compatibility. These are the conditions for signature compatibility:

  • it must respect the variance rules
  • it can make a mandatory parameter optional
  • it can add new parameters, but they must be optional
  • it cannot restrict visibility, but it can relax visibility

Basically, it means that the signature must match. And yours just doesn't.

How to refactor? I would refactor to an interface. Something like:

``` <?php

interface ContentCreatorConstraintsInterface { }

class ContentCreator { protected ContentCreatorConstraintsInterface $constraints;

public function create(ContentCreatorConstraintsInterface $constraints): bool
{
    $this->constraints = $constraints;

    echo 'Slug: ' . $this->constraints->slug . PHP_EOL;
    echo 'Content Type: ' . $this->constraints->contentType . PHP_EOL;

    return true;
}

}

class UserContentCreator extends ContentCreator { private ContentCreatorConstraintsInterface $userConstraints;

public function create(ContentCreatorConstraintsInterface $constraints): bool
{
    $this->userConstraints = $constraints;

    echo 'Name: ' . $this->userConstraints->name . PHP_EOL;
    echo 'Address: ' . $this->userConstraints->address . PHP_EOL;

    return parent::create($constraints);
}

}

class ContentCreatorConstraintsDTO implements ContentCreatorConstraintsInterface { public string $slug; public string $contentType; }

class UserCreatorConstraintsDTO extends ContentCreatorConstraintsDTO implements ContentCreatorConstraintsInterface { public string $name; public string $address; }

$userCreator = new UserContentCreator(); $userCreatorConstraintsDTO = new UserCreatorConstraintsDTO(); $userCreatorConstraintsDTO->slug = 'user-slug'; $userCreatorConstraintsDTO->contentType = 'user-content-type'; $userCreatorConstraintsDTO->name = 'user-name'; $userCreatorConstraintsDTO->address = 'user-address'; $userCreator->create($userCreatorConstraintsDTO);

```

There are some things I don't love in this code, but that is mostly because I don't know what the purpose of it is. That said, I created an empty interface because ContentCreatorConstraints and UserCreatorConstraints are really DTOs. As such, I removed the getters and setters. After all, if you just expose your class properties like that, what even is the point of encapsulation? Make your life easier and just make those properties public already. Type hint the interface. Have the DTOs implement the interface. Bam. No more issues.

RealWorldPHP

Or, instead of checking for the 30 row limit, just check the file size. You don't have to read in the whole file into memory to do that, and it would let you know if it was too large to deal with.

https://www.php.net/manual/en/function.filesize.php

RealWorldPHP

You say that it reaches your function for sure. If so, then it is not a webserver error. In that case, I would think it is either the `findOrFail` call or the response eloquent call.

If either db call is not found, I think you would get a 404. Test to see if your code after the findOrFail is being executed. And then test to see if the Advisor::...get() call is working. Maybe remove the "with()" method and see if it runs.

RealWorldPHP

This. The League has a lot of great packages, and it would be the first place I would look. If the OP wants an out-of-the-box solution that will fit their exact situation, then it will be very difficult to find anything suitable. But it can be coded to work for any CSV issue using this package.

RealWorldPHP

This is a difficult question to answer because, like almost everything in programming, there is no "best" -- there are only tradeoffs. So, the answer is usually "it depends."

  1. I don't know of any rule that says how many action classes a controller should depend on. This depends totally on you and your team and how your code is structured. Maybe 0 is the right answer, maybe 10. Only you and your team can say what is right for you all.
  2. It looks like you are using Laravel. This is some sort of resource controller, I'm guessing. So the controller should be doing one thing: managing your resource, in this case, subcategories. Your action classes should be making life easier for you. It should simplify your controller, right? You don't want to move all of your complexity from one class to another class because what have you accomplished then? Splitting up your action classes is probably the way to go. You have one for GetSubcategoryAction and one for CreateSubcategoryAction, and that makes sense. Each one has one job and those jobs are simple and it simplifies your method. Laravel will let you inject directly into the method, or if more than one method will be using those actions, you can inject into the constructor.
  3. You decide if you want an action class for select queries. Maybe it would keep you consistent. Maybe it is over kill. No wrong answer here, I think.

Using action classes is fine. It works to simplify your controllers and moves business logic away from the framework. Or at least, it can. Have you looked into domain driven design and layering your application? There is always more to learn, and the more I've learned, the more I realize that there is no silver bullet or perfect approach that solves every problem in every situation.

Good luck.

RealWorldPHP

There is no "gold standard" front end for Laravel. It does not matter what front end you use: Angular, React, Vue, jQuery, Ember, Alpine, etc. All the same. There is no down side to choosing React over Vue. You can dive in with no hesitation and no regrets.

RealWorldPHP

Nice job on getting the 23:59 thing to do what you wanted. I would agree with what other people are saying about the dangers of modifying the core code and how it makes future updates difficult. But that said, any knowledge you gain as you figure this all out will be hard earned. And nothing in life that is worth having comes easy. Doing these sorts of things will have a lot of upsides: you will understand a complex system from the inside, your programming skills will greatly improve, your confidence in being able to accomplish hard things will go up, etc. I admire your dedication here.

A couple more thoughts...

  1. Since it is an open source project, there is an issue tracker where other people have documented not only bugs but also feature requests. So you have an option to request the feature you want from the core developers. Of course, even if they like your idea, it may be a long time before it ever gets implemented.
  2. Since it is open source, once you figure out what you want to do, if you think others would like it, you can submit your code change as a pull request in github, and there is a much better chance of getting your change in a future version of moodle. This will require an understanding of Git and Moodle's change requirements, but those are just more hills for you to climb in your journey as a programmer.
  3. Again, I don't know the codebase, but I would guess that to do what you want to do you will need to modify the theme. This may be PHP code or may be html and javascript. But essentially, somehow data is being passed to whatever is going to display what you see in your browser, and you have to figure out if what is being passed contains what you want (the due date). If it does, you just display it. If it doesn't, you have to figure out how to include it in what is being passed to the view.
RealWorldPHP

Full disclosure, I don't know the codebase, but I used Moodle a bit over ten years ago. Moodle is a mature product. It has been around for many years, and as you say, the code is quite complex. Do you already know PHP or are you a beginner? If you want to make modifications, it will take in-depth knowledge and understanding of the code itself. If I were you, I would start with the developer documentation: https://docs.moodle.org/401/en/Development:Developer_documentation

Good luck.

RealWorldPHP

Using error suppression is a code smell.

  • Like you said, you can suppress errors that you didn't mean to suppress
  • It makes debugging way more difficult
  • There are already good ways to handle errors like exceptions and error handlers which will gracefully deal with errors without hiding them

To your exact question about simple situations, I would still avoid using the error control operator.

  • First, no one programs perfectly all the time, and a situation will invariably arise when you think it is impossible to accidentally suppress an unintentional error and you will and you won't know why your code doesn't work.
  • Second, in a given situation where you want to reach for error suppression, it is probably a situation where you can improve your code. Why is it that `$my_array` might not be set? Could you benefit from type hinting? Maybe $my_array should be a DTO or a value object.
  • Third, co-workers will see this and think it is a red flag first and not realize you are trying to save writing twenty characters of code.

At least, that is my take on it.

RealWorldPHP

Fair point.

I would also check out PHP Deployer (https://deployer.org/) as an open source/free alternative to some of those paid solutions mentioned in other comments.

RealWorldPHP

Right, but note how u/pyr0t3chnician uses a symlink to switch things over to the new build. So if things go badly during any of the previous steps, it won't affect what is currently up on production. With the symlink, switching to the new build should be pretty much seamless for the end user.

RealWorldPHP

I think this is a subjective question. What is the right kind of test? For me, a unit test should prove that the SUT (subject under test) behaves in a particular way. In your case, AccountControllerTest should show how AccountController works. It should show the different ways that AccountController can be used, and what will happen if it is misused. If you have done that, then you have the right kind of test.

Full disclosure, I don't know Pest. I use PHPUnit at work and have done so for 10+ years. But it seems to me that your tests are descriptive and don't try to do too much in each test. Those are positive signs. Keep on writing tests!

RealWorldPHP

Under the hood, Laravel uses Symfony's VarDumper, and just passes the arguments from dd() to dump() and then it calls die().

VarDumper is complex and nuanced. It takes into consideration the context of the call (i.e. is it being called to output to a browser or the command line, etc.) as well as what is passed (i.e. is it a string or array or whatever).

But if you just wanted something super simple, I think that vanilla PHP equivalent from u/benzilla04 is not bad at all. My approach would be:

<?php
function dd($data) {
    die(var_dump($data));
}

(var_dump() instead of print_r() in case you aren't outputting to a browser) But, in the end, it is pretty much the same thing.

RealWorldPHP

It really is helpful in a lot of ways. As long as you don't use it in production, it is a tool that should be in most PHP Dev's toolbox.

RealWorldPHP

I really like using die(var_dump($var)); It is great. Also, as u/geekette1 says, xDebug is excellent, although it may be overkill for simple stuff. (There is an argument to be made that every PHP developer should learn to debug with xDebug eventually.)

You can also try something like http://phpdebugbar.com/ which maybe will be somewhere in the middle.

RealWorldPHP

Laracasts isn't good. It is great. It is the de-facto gold standard for learning Laravel since its inception. Period.

RealWorldPHP

I don't think there is any best practice associated with this situation, per se. There are lots of ways you could achieve what you want. Ultimately, I think you should go with whichever way is the easiest to understand.

There is a principle called "Cognitive Complexity" that I think applies here. It is the measure of how difficult code is to intuitively understand and should not be conflated with Cyclomatic Complexity. The point is to make your code as easy to read and understand as possible. Your future self and other future developers that see your code down the road will thank you for keeping this easily understood.

RealWorldPHP

Inversion of Control is a design principle. It is an approach to achieve loose coupling by redirecting the flow of control. Dependency Injection pattern is a type of IoC, but IoC isn't always DI. It doesn't make sense to say that Java natively supports IoC and PHP doesn't. Both Java and PHP fully support Object Oriented Programming. And both have the same approach to Inversion of Control. That is to say, neither language has a built-in procedure for IoC. So for both language, dependency injection containers are written to help with IoC: like Spring has a DI container or Google Guice and Laravel has its Service Container or there is the PHP-DI proejct. Of course, you don't need a dependency injection framework to do dependency injection. You can do it manually, but having a framework is helpful when you are building an app.

I don't personally know any programming language that "natively supports Inversion of Control" but I could be wrong.

I do not know what you mean by "capsulated." If you are referring to "encapsulation" as it relates to OOP, that has nothing to do with Inversion of Control. It has to do with visibility into an object and what is allowed to change an object's properties.

RealWorldPHP

Note on the env() helper point: the reason (which is in the docs) why it is best practice to only use env() in config files is because configs can be cached, and when they are cached, the env() function will return null.

RealWorldPHP

I like this question. And I agree that the Service Provider documentation doesn't explain why you would need to use a Service Provider. I think the answer to you question is in the docs, but it is in the Service Container section. It is absolutely worth a good perusal, but here is my take on why to use Service Providers.

It all comes down to dependency injection. There are a lot of reasons why DI is a good idea, and in your example, you use it in your MyController::Index() method. When you inject your MyLib class into the Index() method, the Service Container is automatically resolving the call for you. And for any class that you want to inject somewhere, the Service Container will do that, provided that it is a concrete class with easily resolvable construction parameters.

But what happens when what you want to inject is more complicated than your simple class? This is when you need to register the class with the Service Container so that it knows how to correctly construct your class so it can be injected.

Examples:

  • When the class needs to be a singleton. A singleton should only have one instance, you can tell the Service Container to only ever have one instance so it won't try and create a new instance every single time it is called.
  • When the class has a construction parameter that is an interface. It cannot be automatically known which concrete class that implements the interface should be used. If you have a couple of classes that implement Traversable, and your constructor has as its parameter something like public function __construct(\Traversable $t) {...}, then the Service Container needs to know more information to resolve that interface.
  • When you want to resolve an interface. This is different from above. What if you want to just inject an interface? Laravel's Service Container will let you. You just have to register it in a Service Provider, and you are good to go. This is one more way that interfaces are powerful.

The Service Container can handle more situations than those I put as examples. It all gets registered in the Service Provider. Service Providers register all kinds of stuff by default, and are super helpful for devs to instruct the Service Container on how to resolve stuff. That is why we use them.

RealWorldPHP

Exactly.

In case OP doesn't click on the link, here is the function definition of strpos:

strpos(string $haystack, string $needle, int $offset = 0): int|false

Note the order of the parameters. The string you are searching through goes first, and what you are looking for goes second. The third parameter is optional. And if it fails, it returns false. When you echo false; you get no output, which is what you are seeing in your code.

RealWorldPHP

Professionally, I am currently using CakePHP 4 and Laravel 8. I much prefer Laravel or Symfony to CakePHP, but sometimes you don't get to choose.

RealWorldPHP

I have an AS in Comp Sci, but I feel like my real programming education started when blogs were first coming out and I thought, "hey, I bet I could program my own blog." 20+ years later, I'm still doing web development as I made a career out of it.