Level 75
Why would you do that when DB can be used Direct inside a controller.
Mar 15, 2018
12
Level 15
Classes with only static methods
Is it good or bad practice to have a class with only static methods? To be more precise methods like this...
public static function foo(){
$bar = \DB::select('SELECT something FROM dbo.table');
// do something with $bar
return $bar;
}
...
Then I am using this methods inside my Controllers, Jobs etc...
Level 15
I want clean and readable code inside my controller... Problem is that I don't own databse, I just read data from it... So... Realy long queries with some formating + some additional logic... If I put that inside controller everything looks messed up...
Level 3
Bad practice. Why wrap in a class if they are static? You instantiate a class to create an object hence object orientated code. Could perhaps be described as a anti-pattern or just bad practice.
1 like
Level 15
@CJJ Do you have any advice where to put those methods instead?
Level 1
those methods should be in services as the rule said thin controllers, fat services https://laravel.com/docs/5.6/container
Level 3
@shime Good question and I don't have a definitive Laravel answer but I would probably take each method on it's own merits.
I will use static methods driven by "users" inside the "user" model and many other variations of the same pattern. Private methods inside a controller for isolated functionality.
The idea of a class that only contains static functions feels wrong. Why are you grouping methods by "static" functionality? Some people may disagree or prove me wrong.
Can you be more specific in your examples/bigger picture? There maybe other patterns you are missing.
Also, the example you provide looks like is could be better written using the Eloquent Query Builder https://laravel.com/docs/5.6/queries .
Level 23
thats why people have repository pattern - you dont generally need a service unless you are trying to work with more than one repository at the same time for example creating a product means creating its variants, images, etc etc.. this will be as ervice otherwise use repository pattern
which is same as what you are doing but more DRY and better so you can repeat the code for diff repositories..
If you want to hide the complexity of SQLs then there is no harm in doing static methods... but group functions into separate class according to the table they act on. BUT considering we have MODELS/ELOQUENT why dont you use these?
1 like
Level 15
@shez1983 There are only one or two methods per table. But somebody inserted data without id, name or anything similar... Just values and some random strings... I litteraly had to find records positions inside the tables, and then use that indexes inside my functions... So in conditions like that there is no any benefit from using Eloquent ORM (except Model::all() ). Since I am using:
\DB::select('SELECT something FROM dbo.table');
inside every function, I know that i am repeating myself, but writing a method that for example accepts $fields and $table wouldn't help me. From some of tables I select 10+ fields named name1_name2_name3_i, so I would have to write something like:
name1_name2_name3_10 AS foo_10
for each field. And then pass all that together as an argument. So basicly it is same thing as using \DB facade inside my controller.
Level 1
This is just a terrible idea. You need to learn more about Laravel and just PHP in general if you think it's a good idea to be doing these kinds of things. If you're so bent on selecting specific things from a table, it can easily be done with Eloquent. There's no need to use the DB facade.
You can do this instead: SomeModel::get(['column1', 'column2', column3']); Wow! Look how much simpler that is. With that way, you don't need to specify the table or write the query yourself. If you're still concerned about writing that again and again, then use a repository, where you can just pass an array of the columns to select in.
Level 15
@gideon I don't want to name columns as they are named inside db... So:
$bars = SomeModel::get(['column1', 'column2', column3']);
foreach( $bars as $bar){
// do something with $bar -> column1 is not what I want
// I want to do something with $bar->myColumn1
}
Generaly, I think that there is nothing wrong with the queries I wrote, and my question wasn't about that. I just wanted to know is it good or bad practice to put those queries inside static methods in separate class in order to get cleaner code inside my controller. And let's say that all of you agreed that It's a bad practice.
Level 1
@shime - You're just making more work for yourself. You're a fool if the only reason you are aliasing the columns is because you want to.
But since you seem so bent on doing that, it's possible to make a method where you pass your columns in where you do something like this: SomeModel::select($columns)->get();. But again, it's just a bad idea to do that.
As far as the static methods go, just why? Why even have a class if all it has are static methods?
Level 15
@gideon Sorry, I haven't seen you response. I am aware of that... But on the other side if I name columns as I want, I can use the same name in all of my views... Anyway, It seems everybody just keeps saying it is wrong, but I am not sure that everybody knows why it is wrong... Thanks for answer(s)... :D
Please or to participate in this conversation.