New answers tagged php
0
I don't really understand the purpose of this class.
In my understanding, there is no output before all the business logic finished.
Then, if no errors occurred, a template is rendered.
In case there are validation errors, they are collected and then a template is rendered, displaying validation errors along with the corresponding form inputs.
In case ...
0
This looks very clean, there is not much to improve upon.
However, there is a glaring security issue:
https://www.owasp.org/index.php/Cross-site_Scripting_(XSS)
Your code should not allow malicious JavaScript to be stored inside of the db.
1
The biggest thing I see are the numerous SQL injection vulnerabilities enabled by concatenating variables with SQL together. Use Prepared Statements instead.
You can reduce nesting in code by returning early from the function.
$query = $this->db->query($sql);
if ($query->num_rows() == 0)
return false;
$sql = "insert into tableB (idno, aid) ...
4
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 purpose of the methods is, or the content of the variables.
Your method getDocument() can do two things: Return the path or the folder of the document. The name ...
1
Fight the urge to squeeze multiple "things" into one line. Your script will be easier to read and maintain with all declarations and constructs occupying their own rightful place. Spend the extra lines, you'll be happy you did a year from now.
Use curly braces to encapsulate language constructs (e.g. if-else and foreach, etc.), this combined with ...
1
The best performing techniques for filtering arrays employ key-based comparisons.
To prepare your posted data structures for such a technique will require both arrays to have there ids copied to the first level keys.
Once both arrays have id values for keys, you can filter by comparison.
Code: (Demo)
$fruits_arr = [
['id' => 213, 'fruit' => '...
1
There's nothing wrong with your foreach loop, but you can use a slightly different syntax:
$selected_fruits = [];
foreach ($fruits as $fruit) {
if (in_array($fruit["id"], $selected_ids)) {
$selected_fruits[] = $fruit;
}
}
print_r($selected_fruits);
I've changed $fruits_arr to $fruits, and $ids_arr to $selected_ids. A variable name should ...
2
Only thing you can use besides the foreach loop, it is array_filter:
$selected_fruits = array_filter($fruits_arr, function($fruit) use($ids_arr) {
return in_array( $fruit['id'], $ids_arr );
});
looks shorter and uses native functions
1
Neither you .htaccess file or your .php file would be Vulnerable to Xss Attacks as they are both server side scripts which would never be Shown to a user & therefore would not be easily manipulated.
to add a layer of protection for your site against Xss look over the following snippet.
In the .htaccess file that is in the main root directory of your ...
2
You could make your code "more dynamic" by creating a whitelist of "actionable keys" and the associated method name to be called, but that seems like unnecessary convolution.
Your current method is direct and literal and probably makes sense to those on your development team. I only have a few minor suggestions, but overall you should keep your basic ...
1
Subclassing
What's the point of this code? Since __CLASS__ always refers to the class in which the code appears, rather than the class of the current object, this method doesn't work with subclassing. That is, with the following code, (new SubModel())->getModelName() would produce base_model, not sub_model:
class BaseModel {
public function ...
3
You tagged laravel, so this answer assumes you are using Laravel.
Laravel has two built in helper functions that will help you do this. class_basename() will return the name of the class with the namespace stripped off, and snake_case() will convert a string to snake case.
Combine them, and you get:
public function getModelName()
{
return snake_case(...
1
Efficiency & Clarity
For efficiency the only "flaws" (it's more for "good habit" than noticeable optimization - it doesn't matter for such a small strings) would be the order of functions (regexp is costly so it should be called on string processed as far as possible) and regexp itself which could be optimized for least amount of steps.
When it comes ...
1
Yes, you can certainly nest your checks a bit better to improve DRYness.
isset() has an advantage over empty() in this scenario because isset() accepts multiple parameters versus empty() only permits a single parameter.
using a chain of null coalescing operators will reduce the lines of code near the end of your custom function. Here is a reference to this ...
1
To convert $cats into a lookup array, you merely need to assign new 1st level keys using each subarray's category_style_code value. array_column() allows this nifty trick.
Once your lookup array is prepared, then you only need to loop through the $prods data. When finished (and if necessary for your project) re-index the output array to remove the ...
1
I won't go down the rabbit hole of debating what constitutes a "valid Pascal/Studly-cased string" but there is plenty of debate here if you want to read about fringe cases (like acronyms and multibyte characters). I will merely cross my fingers and hope that your project's naming convention does not wonder into tricky territory. Either way, the post-...
1
There's actually not much I can do when it comes to efficiency, but it is clear that the code is not very readable. All these nested function calls make it very hard to track what's going on.
My suggestion would be to split this single method into multiple methods. This is a very normal strategy in programming. A method should only do one thing, and do it ...
2
I agree with all that mickmackusa has said, but I have four additions:
The return value must be defined for cases where the values in the yearly array add up to less than the total value that was spent. I chose to return false for this. So when the function return false it means that it cannot compute the payback years, given the input.
When the total ...
4
You should not be calling sizeof() after every iteration. In fact, using foreach() eliminates the need to count at all and provides an incrementing counter variable.
When you want to break out of a loop and immediately return from your custom function, just use return.
Notice in my code below that it is not necessary to write an elseif or else condition.
...
3
It is a good thing you completely seperated your testing code from the code you're testing.
All your tests have the same basic structure, so why not create an array containing:
[1 => "I",
2 => "II",
3 => "III",
4 => "IV",
5 => "V",
............];
And use that array to run your tests. You could even use the same array to test a ...
1
After a bit more consideration and soul searching, I suppose the correct technique to use would be the one that avoids an iterative "guess and check" technique and avoids changing the input's data type.
The input data type is a string and the output data type is a string; I don't find it very compelling to convert the input temporarily to an array just to ...
1
In case you'd like to compare/benchmark the regex technique, it will look like this:
Code: (Demo)
$params = "colorgroup-b;test;abc";
var_dump(array_includes("colorgroup", $params)); //outputs: colorgroup-b
function array_includes($needle, $params) {
return preg_match('/[^;]*' . preg_quote($needle, '/') . '[^;]*/', $params, $m) ? $m[0] : false;
}
...
2
Your function checks if an array contains a string that contains your given $needle. No, there's no such function in PHP yet. The in_array() function does something else.
You could make your function more efficient if you include the explode() inside the function, like this:
function find_parameters($needle, $parameterStr)
{
if (strpos($...
0
Passing the database connection as a variable is a good technique.
Validate user input as much as possible before considering executing any processes. If the input is missing or invalid, don't do anything with the submission.
if (empty($_GET["user"])) { // add your validation here if possible
echo "Missing/Invalid data submitted";
// halt the ...
1
First of all, again, it's a very good approach. You are going in the right direction. Such a separation is a cornerstone of the data manipulation.
Regarding the particular implementation,
for the DB class I would suggest 2 things
I don't think this approach with arrays in $params is really needed. I bet you made it just because PDO has such parameters, ...
4
I was about to write an answer almost literally the same as written by KIKO Software (even with the same hyperlinks), so I would only elaborate on it (and somewhat make an answer for the generic question from the question title).
Both queryDatabase() and getResult() functions from the other two answers are too specific. They cut off the PDO's great ability ...
6
If I assume that leave_detail_id contains an auto-increment value, starting at 1, you can do:
function queryDatabase($query, $bindings = [])
{
global $connPDO;
// prepare statement
$statement = $connPDO->prepare($query);
// execute statement
$statement->execute($bindings);
// return result in associative array
return $...
2
The code below reduces the duplication you have.
The SQL statement is the first thing that is duplicated, with the exception of the where clause. this means we can extract the building of the query to a function that accepts the where clause and returns our query.
The result parsing is similar, accepting two parameters instead of one. Rather than just copy ...
0
If statements without brackets work but it's hard to read and will trip you up eventually.
if (!empty($cookieId))
$cookieExists = true;
This code is redundant
if($agent->isDesktop()) {
$data['isDesktop'] = true;
} else {
$data['isDesktop'] = false;
}
and can be replaced with
$data['isDesktop'] = $...
1
Two things here,
Remove your extra empty lines as its hard to read.
Use PHPDoc comments for each function, it's like JavaDocs but for PHP and you can use it to generate documentation for your classes. So Like
/**
* Save the error to a log
*/
private function saveToLog(){
instead of
/* Save the error to a log
===========================================*/
...
2
That's a pretty solid implementation, with many possible issues already solved. However, there is room for improvement still.
Here is a brief list:
Yes, you are absolutely right, a db wrapper must be separated from the data mapper, with insertRecords, updateRecords and deletedRecords moved into the latter.
Your database wrapper offers less features than ...
1
In your action processing script, you should, before doing anything else, validate the input. If anything is outside of the expected range of values, you should immediately return a failure response.
I will also urge you to write your query functions in object-oriented syntax rather than procedural because it is more brief and easier to read (IMO).
Since ...
Top 50 recent answers are included
