C
C
cannabioid13372022-01-21 20:44:16
PHP
cannabioid1337, 2022-01-21 20:44:16

How can you shorten the shit code?

This horror has already stung as much as possible, but still the feeling that this code is very dirty.
How can you make it more beautiful?

public function index(Request $request)
    {
        // проверки

        if ($request->input("type") == "bet" ||
            $request->input("type") == "withdraw" ||
            $request->input("type") !== "create") 
        {
            if (!Auth::check()) {
                return json_encode(["errors" => "Вы должны быть авторизованы"]);
            }
        }

        if ($request->input("type") == "bet" ||
            $request->input("type") == "create") 
            {
                if (User::getUser()->balance < intval($request->input("bet"))) 
                {
                    return json_encode(["errors" => "Недостаточно средств"]);
                }
            }
        //

        $game = $request->input("game");

        switch ($game) {
            case "dice":
                $validator = Validator::make($request->all(), [
                    "bet" => "required|numeric|min:1|max:10000",
                    "coef" => "required|numeric|min:1.05|max:100",
                ]);

                if ($validator->fails()) 
                {
                    return json_encode(["errors" => $validator->errors()->first()]);
                }

                $this->bet = intval($request->input("bet"));
                $this->coef = floatval($request->input("coef"));

                return $this->dice();
                break;

            case "coin":
                if ($request->input("type") !== "bet" && $request->input("type") !== "withdraw") 
                    {
                        $validator = Validator::make($request->all(), [
                            "bet" => "required|numeric|min:1|max:15000",
                        ]);

                        if ($validator->fails() && $request->input("type") !== "is_active") 
                        {
                            return json_encode(["errors" => $validator->errors()->first()]);
                        }
                    } 

                $this->bet = intval($request->input("bet"));
                $this->side = $request->input("side");
                $this->type = $request->input("type");

                return $this->coin();
                break;

            case "mines":
                if (
                    $request->input("type") !== "bet" &&
                    $request->input("type") !== "get_coefs"
                ) {
                    $validator = Validator::make($request->all(), [
                        "bet" => "required|numeric|min:1|max:5000",
                        "mines" => "required|numeric|min:3|max:24",
                    ]);

                    if  ($validator->fails() &&
                        $request->input("type") !== "is_active" &&
                        $request->input("type") !== "withdraw") 
                        {
                            return json_encode(["errors" => $validator->errors()->first(),]);
                        }
                }

                $this->bet = intval($request->input("bet"));
                $this->mines = $request->input("mines");
                $this->mine = $request->input("mine");
                $this->type = $request->input("type");
                return $this->mines();
                break;

            case "wheel":
                $validator = Validator::make($request->all(), [
                    "bet" => "required|numeric|min:1"
                ]);

                if ($validator->fails() && $request->input("type") == "create")
                {
                    return json_encode(["errors" => $validator->errors()->first(),]);
                }

                $this->type = $request->input("type");
                $this->color = $request->input("color");
                $this->bet = $request->input("bet");
                $this->game_id = $request->input("game_id");
                return $this->wheel();
                break;

            case "bonus_wheel":
                $this->type = $request->input("type");
                return $this->wheel_bonus();
                break;

            default:
                return "unknown game";
        }
    }

Answer the question

In order to leave comments, you need to log in

3 answer(s)
R
rPman, 2022-01-21
@cannabioid1337

Replace heaps with switch case, especially further down the text you use it, write code in at least one style. I would not recommend separating access control, balance from the code for processing the actions themselves, it comes back to haunt you later when you expand the functionality and forget to add a check somewhere higher in the code. Those. everywhere, for each action with a subtype, check whether there are rights, whether there is enough money, and whether you are doing it again, etc. If you are afraid for the inefficiency of such code, checks are made out in the form of methods, inside caching (if, for example, a request goes to the database). Remember that premature optimization is evil.if ($request->input("type")
About access rights, do you have exactly 2 states - public and authorized? maybe you still have some superuser? or else there are rights based on data (for example, if the status of the operation is 'such then' then the action is prohibited) ... although the method for any should be ->allowed() which collects data on the current user, state, data, etc. although it is better to separate access rights by role and by state, let there be a couple of methods.
Here are the lines "required|numeric|min:1|max:10000" - bad practics, magic constants, which means 10k, 15, 3, 24, 5000... different everywhere, why why, after half a year you won't remember, and another programmer will certainly not understand. Make it in the form of a generator function or even a class, document all the values, even if these are values ​​​​from the bullshit, when you invented them, you put some sense into it - describe it in the comments in this function or class, it is not necessary to put everything in the config file , the file itself with such a class is already this config. Not only that, why array serialization is so strange, there is already an unconditional json standard on the market, and machine and human readable, the overhead is minimal (you can force nginx to pack, the traffic will not be much more than a cleverly designed protobuf),

J
jazzus, 2022-01-21
@jazzus

Remove validation in request forms, access rules in policies. The rest depends on logic. See Laravel tools other than controllers and routes.

T
tukbaevbr, 2022-02-01
@tukbaevbr

The author of the code is an idiot.
The questioner is an idiot.
The advisors are fucking idiots (except for JhaoDa ).
All that you have advised about this feces is like a dead poultice.
Can't you see that the main problem with this code is that in one controller action they stuffed what should be in several controllers with all their actions?


Отрефакторить.

               JhaoDa

Didn't find what you were looking for?

Ask your question

Ask a Question

731 491 924 answers to any question