7

I found in Laravel - Eloquent's method like updateOrCreate(). Why in framework we can find methods which are breaking clean code and SOLID rules?

Ezra Lazuardy
  • 406
  • 6
  • 17
JakiLim
  • 227
  • 3
  • 8
  • 2
    Because, sometimes you just don't care if it is one or the other. In both cases, you want to have the record in your database, and it should be unique. If you don't like that, you don't need to use this method. – shaedrich Jun 16 '21 at 18:04
  • 1
    Please clarify which rule you think is broken, and why. – jaco0646 Jun 17 '21 at 20:10
  • @jaco0646 Did you forget this question? – plalx Jul 02 '21 at 12:46
  • 1
    @plalx, the OP seems disinclined to put in any real effort here, so I feel the same way. Your answer is more than sufficient for this kind of thoughtless, drive-by, fishing expedition. I stepped in only because the first answer was so quick to take the OP's vague, unsubstantiated claim at face value. – jaco0646 Jul 02 '21 at 13:24

3 Answers3

2

There's no violation of any principles here and it's not an anti-pattern either, which by definition would be a code smell.

I'm not sure about the specific implementation we are talking about here, but generally speaking an "update or create" operation must be atomic and that would just be cumbersome to implement with distinct create, exists & update operations.

For instance, imagine the client had to write the following code every time he wants to do an "update or create".

if (!exists()) { // this check could be stale without proper locking
   create();
} else {
   update(); // record could have been deleted already
}

As we can see not only that's a lot of ceremony, but proper locking/retry would need to be used to make sure that the record has not been inserted right after we did the exists() check or inversely deleted right before we update().

"update or create" is conceptually a single "merge" operation which cannot be implemented effectively with the client with a bunch of seperate methods. Should they have called it merge you wouldn't have asked this question ;)

It's not because there's an "or", "and", "ifNeeded" or whatever in a method name that it necessarily has too much responsibility. It's a strong indicator it may be the case, but use your own judgement and do not follow principles blindly.

plalx
  • 42,889
  • 6
  • 74
  • 90
  • I like this answer, but I think the first and last sentences are at odds. The first sentence is spot on: there is no violation of anything here (assumptions like the one in the OP are often based on some [gross oversimplification](https://stackoverflow.com/a/317294/1371329)). Because there is no violation, there is no need for judgement. Furthermore, effective judgement requires understanding. Lack of understanding is the problem in the OP, rather than lack of judgement. – jaco0646 Jun 24 '21 at 14:28
  • I know this is kind of out of the context, but how can you update the same value that you're using to check for matching, in the 1st paremeter of the updateOrCreate method. – Wakil Ahmed Sep 15 '22 at 14:22
  • @WakilAhmed You'd read a copy first which is what's compared against the updated state. Not sure if I properly understood the question though, could you be more specific? – plalx Sep 15 '22 at 15:16
  • @plalx You know in updateOrCreate method we pass two parameters. The first parameter is used to check for a match and the second parameter carries the data that needs to be updated. But in my scenario I need to update the same data that I'm passing in the 1st parameter. But I can't update it If I pass the same in the second parameter. – Wakil Ahmed Sep 16 '22 at 05:11
1

I think you're referring to the Single Responsibility Principle as stated in SOLID. Long story short, yes, some methods in Laravel PHP framework breaks SOLID, and not just in the Eloquent ORM.

This is called an Anti-Pattern, as stated in Wikipedia:

An anti-pattern is a common response to a recurring problem that is usually ineffective and risks being highly counterproductive.

The reason we use this "anti-pattern" is just for the sake of simplicity and productivity. Depends on your application, handling data with ORM (such as Eloquent) sometimes can be complex, and this is why anti-pattern come to rescue.

With eloquent, you can use create() to just add a new record and updateOrCreate() to add a new record or update it if it already exists.

Ezra Lazuardy
  • 406
  • 6
  • 17
  • I suspect there is some confusion here regarding the meaning of the SRP. Can you clarify how the principle would be violated in this case? – jaco0646 Jun 17 '21 at 20:17
  • I was confused too regarding which principle is violated at first, but I think what JakiLim meant was why some method can perform many functionalities in Laravel. That's why I refer to SRP because it's the only principle that states `every module, class or function in a computer program should have responsibility over a single part of that program's functionality` (reference: [Wikipedia](https://en.wikipedia.org/wiki/Single-responsibility_principle)). Feel free to correct me if im wrong :) – Ezra Lazuardy Jun 17 '21 at 23:00
  • Unfortunately, Wikipedia is just as confused as the OP. It is a very poor source for software design since it is edited repeatedly by unqualified authors. [What is the scope of the Single Responsibility Principle?](https://stackoverflow.com/a/57085753/1371329) should clear up some of the confusion. – jaco0646 Jun 17 '21 at 23:13
  • Yes, that's confusing. Since this topic is opinion-based, I'll just add your link as reference in my answer. Thanks. – Ezra Lazuardy Jun 17 '21 at 23:18
  • Please don't do that. The link contradicts this answer. The two do not fit together. My point was that the OP is based on a faulty premise and therefore is invalid. This attempt to answer an invalid question is itself based on incorrect information. The link does not support this answer. – jaco0646 Jun 17 '21 at 23:29
  • 1
    This is not an anti-pattern at all, see my answer. – plalx Jun 24 '21 at 04:18
0

I found out in Illuminate/Database module of Laravel framework break a lot the Liskov substitution principle rule in SOLID as well. I'm not sure why the author did that also.

VuVanLy
  • 69
  • 6