Hot answers tagged

9 votes
Accepted

Phishing Project Assessment

✔ Using environment variables This is a great way to reduce the risk of credentials being leaked through version control. Kudos. ✘ No PHPDoc comments Although pedantic, your method comments aren't ...
ʰᵈˑ's user avatar
  • 336
7 votes

Queries to get unpaid and unpaid invoices

There seems to be a section in your code that you repeat three times. This can probably be placed in a separate function. Something like this: ...
KIKO Software's user avatar
6 votes
Accepted

A Search Engine Class

I really like the structure of your code. That said, I almost never suggest to give class variables default values. That is what the constructor is for. For that matter, you don't need to specify that ...
Trojan404's user avatar
  • 818
6 votes
Accepted

Setting default of <options> in Laravel with PHP Match

Some variable assignments are excessive Let's look at what $selected is set to: $selected = 'selected'; Then in both of the <...
Sᴀᴍ Onᴇᴌᴀ's user avatar
5 votes
Accepted

Phishing application God class

Security Try to follow current best practices. If you are very new to web security, take a quick look at the OWASP Top 10. SQL Injection You are open to SQL Injection via the ...
tim's user avatar
  • 25.1k
5 votes
Accepted

Phishing Project Error Logging

DB Manager thoughts: 1) I am generally not understanding what you are trying to achieve by even having this class. It is not doing anything like managing database connections, abstracting the user ...
Mike Brant's user avatar
  • 9,598
5 votes
Accepted

Simple inbox functionality using JavaScript and Ajax

Style guide I'd recommend picking a style guide to keep all of your whitespace consistent. I've formatted yours according to semistandard. Having a style guide could be considered a form of bike-...
Dan's user avatar
  • 3,738
5 votes
Accepted

Document Merger using PhpDocX

Security: Credentials in ENV It is less than ideal to store your credentials in environment variables, as they may easily leak. phpinfo for example will print all ...
tim's user avatar
  • 25.1k
5 votes

Laravel 5.8: prevent duplicated code

I think you can DRY this up pretty well by refactoring. This feels like a “ifs are evil” code smell to me. There are three “if” statements within that whereHas. They are packed in fairly tightly, and ...
James Clark's user avatar
5 votes
Accepted

How to reduce a POST request duplicate in Laravel/PHP for payment system?

Let's implements following: separate logic, refactoring of methods Separate logic Validate Requests/PaymentRequest.php add rule for validation ...
Kostiantyn Okhotnyk's user avatar
5 votes
Accepted

Update specific model attributes from a request

How does this look? It looks okay. It appears to mostly adhere to PSR-12 and is easy to read. While there is only one method with a single loop it isn't very complicated. The variable names are ...
Sᴀᴍ Onᴇᴌᴀ's user avatar
4 votes

Phishing Project Assessment

...
Pimgd's user avatar
  • 22.3k
4 votes

Simple inbox functionality using JavaScript and Ajax

From a short review: I hope your id's are not simply an increasing number, otherwise this: ...
konijn's user avatar
  • 33.1k
4 votes

Populating a form with filtered hierarchical place names

There are some major concerns with this approach. First, why rebuild all the DOM elements every time there is a change? Your universe of different countries, provinces, districts, etc. is static ...
Mike Brant's user avatar
  • 9,598
4 votes

User Avatar upload

I honestly don't think there is much to "clean up" here, as this seems pretty straight forward. A few stylistic notes: It is pretty standard in PHP to have variables named with first letter in lower ...
Mike Brant's user avatar
  • 9,598
4 votes
Accepted

Automated Tweet of new inbound Enquiries in PHP7 (Laravel 5.2)

Code intent The main problem with your code is that it does not give any reasons as to why it performs the way it does which makes understanding, reviewing or upgrading it difficult. ...
arcanine's user avatar
  • 364
4 votes
Accepted

Laravel UserController Design

First of all do not use Laravel 5.4 even if you've written some* code already, use latest 5.7, and try to always update. *: You really need to rewrite the application if you have code like you ...
Kyslik's user avatar
  • 181
4 votes
Accepted

A PHP Controller, which feels to 'array-like', in Laravel

You could use route model binding to remove some lines from your code, but that would require changing the route so it passes the MenuItem's id. ...
IGP's user avatar
  • 158
4 votes
Accepted

A method to get a document's path

I'm sorry, this is not an answer to your question, but it is a partial review of your code. It seems you've got a serious problem with choosing your names. They don't actually tell me what the ...
KIKO Software's user avatar
4 votes
Accepted

Using PHP goto Labels for code folding

The main issue I have with your approach is that it is non-standard and purely for the way that you work. Others looking at your code may be a bit confused as to what you are doing and why. In your ...
Nigel Ren's user avatar
  • 338
4 votes
Accepted

Laravel controller method that searches jobs using variable criteria

I'd like to recommend condensing some of your script so that it is easier on the eyes. This: ...
mickmackusa's user avatar
  • 8,574
4 votes

Refactor PHP | Laravel Code, Optimisation needed to make code clean

You could exploit Laravel collections to achieve what you need. Most of your code is checking for the presence/absence of props on your arrays. Laravel internally exposes and uses the ...
mdexp's user avatar
  • 141
4 votes
Accepted

What would be the best way to update data from associative array?

"Premature optimization is the root of all evil." As long as this code works and causes no problem, nothing should be really done. Trying to "improve" it, most likely you will cause a severe harm to ...
Your Common Sense's user avatar
4 votes

Structuring code logic for events on laravel controller

I'm mostly a back-end guy who fusses his way through front end code. Here's my advice: Keep the controller code as close as possible to handling the response only. Delegate the creation of objects to ...
Nick's user avatar
  • 141
4 votes

Laravel controller method to update user

So the above answer from Sᴀᴍ Onᴇᴌᴀ already covers quite a few points. I've worked with Laravel for the past 4 years, so I'll go a bit further in depth on how I'd approach this, including a few ...
Tropus's user avatar
  • 61
4 votes

Controller method to store course

You could create 2 sub-functions for the different parts in your function. validate($request) so that long part is hidden ...
Double_A's user avatar
4 votes

Optimization of a Laravel controller that passes data and views

I don't use Laravel, but yes, you can greatly reduce the preparatory PHP scripting and make it more "D.R.Y.". You could use a lookup array and then null coalesce to fallback values, use a <...
mickmackusa's user avatar
  • 8,574
4 votes
Accepted

Optimization of a Laravel controller that passes data and views

Put your repeated or complicated queries into scopes on the model class. Here I also use the helper now to avoid importing Carbon: ...
Laurel's user avatar
  • 666
4 votes
Accepted

Retrieve config values using custom helper

I'll review your 2nd and most recent refactor. Your function getTheme() builds a space-delimited string of class values. Your current function name is clear about ...
mickmackusa's user avatar
  • 8,574
4 votes

Laravel controller getting results based on parameter including relationship results

Using eloquent when() function the above code would be cleaner, especially if you add more filters in the future. I am not sure what version of laravel you are using, but Request injection probably ...
Nikolas's user avatar
  • 161

Only top scored, non community-wiki answers of a minimum length are eligible