New answers tagged

0

Let's rephrase it with some rhetorical questions. You want to replace if($result->num_rows){} with the following? $result_set = $result->fetchAll(); $count = count($result_set); if($count>0){} That is, you are asking, should the faster and shorter code be replaced with slower and larger code?


1

Yes, in order to know whether your query returned any rows, in a modern web application it is preferred to fetch the data in a variable first and then use this variable to see whether it's empty or not. In this regard, calling the count() function would be also superfluous. In PHP, an array is as good in the if statement as a boolean value. Also, foreach ...


1

Guard statements if(!empty($_POST['csrfToken'])) { if(hash_equals($_SESSION['csrfToken'], $_POST['csrfToken'])) { I think that this should be performed in a separate method that validates tokens. Furthermore, I think that this should end in an HTTP error (as indicated by the other answer). Above is called a "guard statement", it checks if the ...


1

One way to simplify this would be to create a collection with collect() and then call getName() on each route using Collection::map(). Then use collection::filter() to filter out the routes that don't start with rt_ using the string helper method Str::startsWith() class RouteController extends Controller { public function getRoutes() { // ...


1

Bug / Logic error The two switch statements have one case plus the default case. In both switch statements the default case is always executed. Perhaps a better understanding of how switch statements work would be useful. Also, for a single case it doesn't make sense to use a switch statement - a simple if statement would suffice. Inconsistent return types ...


1

Types It makes the code easier to understand if you specify types for all function signatures. PHP will check types if specified at runtime whenever entering and exiting a function, and when writing to a class property, helping you to detect and find errors much more quickly, especially if using an IDE or static analysis tool. They also help communicate the ...


-1

As we know if want to delete data from referenced table if we delete data from the parent table so for this we use the MySQL cascade feature. We use MySQL ON DELETE cascade in this case. Just take an example if I have two tables. 1- Parent 2- Child and I want to delete referenced data automatically from the child table if I delete data from the parent table. ...


1

Your code is insecure because: It only relies on the file extension, so attackers can upload any backdoors. It accesses variable $_FILES['postToUpload']['name'] without checking if it is defined and this may lead to Full Path Disclosure. It uses function uniqid() without any arguments, so uniqueness is not guaranteed and the files that are uploaded ...


1

Security The code basically attempts to create a Client-side Request Forgery (CSRF) token. Some frameworks like laravel provide this functionality by default. It may not be worth the effort of incorporating a framework if all you need is one feature but if there were others then you might consider it. Generation of a CSRF token would be much simpler than ...


4

Speaking about security, there are several issues: As a rule the attacker will go the easy way: send and analyze first request, remember exerciseText and solution, then send an unlimited number of requests from an unlimited number of devices. So, at least, you should use a timestamp to shorten the token's lifetime and tie it to a specific IP. In fact your ...


4

Remove encryptEasy and decryptEasy; both functions are insecure and only serve to confuse possible users. Your current scheme doesn't use a MAC, not a checksum which means the ciphertext is unauthenticated. This is a bad idea. It is recommended to use a HMAC function for authentication, in order to also avoid the risk of hash extension attacks while keeping ...


2

Your might actually take it a step further and create a value object for the data, making it much more type safe. Notice I added PHP 7.4 proeprty typehints. I chosen float for the temperature (maybe it's not appropriate, idk), but i definitely don't know what to choose for the main, but it sure deserves a typehint, if it's nested object, create another class ...


2

User Interface I noticed that if I entered values into the form and then submitted it, the values were not persisted after the next page load. It would be nice to have the values persisted in the form inputs. This is easy to achieve since the form is rendered by a PHP page. Because the PHP code towards the latter half of index.php comes after the HTML is ...


1

I personally do not prefer the mixing of data collection with data printing, but I understand that this is common on simple Wordpress pages (I am not a Wordpress developer). id values do not need to be purely comprised of letters to be valid, so I would use a counter and scrap the idea of having a limited-length lookup array of words. Using printf() will ...


1

Not sure what you are trying to achieve, but you can use numerical values for enum_of_collapsible_numbers and you can generate them like this $enum_of_collapsible_numbers = range(1, 20);


3

JS and front-end style If you aren't going to reassign a variable, always use const instead of let. Or, since this sounds like it's on a public facing website, if you wish to support ancient obsolete browsers that some users unfortunately still use (such as IE11), either write in ES5, or (recommended for anything more than a handful of lines) use Babel to ...


2

First, here is my recommended replacement for your script (inspired by this SO post) and some test cases: Code: (Demo) function secondsToTime($seconds) { $dtF = new DateTime('@0'); $dtT = new DateTime("@$seconds"); $diff = $dtF->diff($dtT); $units = [ 'y' => 'year', 'm' => 'month', 'd' => 'day', ...


2

The fact that you are making three separate trips to the server stands out as an obvious refinement opportunity. We don't know exactly where your performance bottleneck is, but you can be sure that best practice is to reduce your total trips to ...well, anywhere. Think about it like this: Your mother tells you to go to the shop and get a dozen eggs. You ...


1

It seems that the process of generating an ID is going to be the same for all descendants, it will only differ by the actual class used. So how about doing the work outside the class? abstract class Badge { private string $id; protected array $activities; final public function __construct(string $id, array $activities) { $this->id = $id; ...


0

I would not use the pkav function. Beside the fact that the name means nothing (which was stated in the comments) it also restricts the default value to false. This is wrong when you might expect strings or something else for a certain variable. From the looks of it, the footer text and button vars look like strings. I would go with this <?php $...


3

I would go with lazy loading. Generate the id when requested private $id; public function getId() { if ($this->id === null) { $this->id = //your logic to generate the id here } return $this->id; } and don't use anywhere else $this->id even inside the class. Use $this->getId() everywhere. using $this->getId() everywhere ...


0

Have you tested the server response time independently of your frontend code? The page being slow might not be because of your code being inefficient, if the server or database takes a long time then there's nothing you can do to fix that on the frontend side. That being said, first thing you should do with this code is to look up some standards for PHP and ...


1

In this presentation about cleaning up code Rafael Dohms talks about many ways to keep code lean - like avoiding the else keyword. (see the slides here). It is wise to avoid the else keyword - especially when it isn't needed - e.g. when a previous block contains a return statement: if($response->getStatusCode() >= 500){ return response($...


4

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 their own classes. Think of the S in SOLID - Single Responsibility. public function store(Request $request) { $image = new ImageMeta($base64); $...


3

I suppose I am not convinced that your simple game actually needs to establish any exceptions for these predictable failures in flow. I agree with Greg that a win should definitely not be a throwable exception -- because it is predictable. I like this simplistic answer to "When should you use PHP Exceptions?". Because your script is handling very ...


2

The for loop is not going to be the bottleneck in your loop, but what seems obvious to me is that you don't benefit from counting higher than $repeated = 1. This means that you don't need a counter variable, you actually need a loop breaking event -- in this case die(). For your information, die() will print the text in its first parameter, so using echo is ...


Top 50 recent answers are included