New answers tagged

1

Initial feedback This code looks okay, though has quite a bit of room for improvement - see this suggestions below. I like how the short array syntax is used in many places, as well as type hinting of parameters and return types for methods. Some methods are a bit lengthy and a bit of an eyesore - e.g. the method EvaluateVariable- its more than 100 lines of ...


1

The \x{0020} (SPACE) character is already included in \s, you can omit that part. I prefer to use \p{Cc} over [:cntrl:] for consistency and because I don't like to have nested square braces in a character class. I expect isAttributeNameValid() to be checking that the whole string contains NO blacklisted characters. If you want to match the entire string ...


2

Standards private function AnalysisVariable(string $varname, $var, array $indents): string So there are a couple problems here. In the text of your question, you refer to this as AnalVariable. That's a particularly bad name as anal is an English word that is somewhat off-color. Fortunately changed here. PSR-1 says that method names should be ...


4

First of all, you really deserve to be commended for following best practices in PHP programming. You really want to learn how to develop websites properly and your effort shows. The code which you have shown us already looks amazing and you are on the right track. However, there are still a lot of things that you could improve. Let's review some of them. ...


4

This code is rather good if you look at each separate line, but rather bad from the organizational standpoint. Each separate code snippet is indeed nearly flawless but it's the way they are glued together is wrong. SignUp is, so to say, a "Diety Object" as it's not as omniscient as a God Object, but definitely it does way too much: connects to a ...


3

First of all, your code looks rock-solid, obvious in what it does and very clean. This is not something that I would expect from someone "new in development". Whoever you learned all this from, you should keep them. You got all of the major topics of a login system right: Your code uses parameterized statements for all database access, to protect ...


0

As a general best practice rule, I would advise that any time you are going to be feeding a variable into a regex pattern string AND that variable isn't 100% guaranteed to be free of characters with special meaning to the regex engine -- you should always bake in a preg_quote() call for stability/security. Focusing on match() and paramMatch()... ...


0

Router should not accept request object in constructor. You will need a new router for each request. I know PHP spawns a new process for each request so it gets created for each request anyway. But in tests it may be different... And anyway it doesn't make much sense to hide the request inside the router from the logical point of view. Hiding a constructor ...


0

I fixed it by myself by introducing abstract classes, see below: class Product { private int $id; private string $name; private int $price; private Category $category; public function __construct(int $id, string $name, int $price, Category $category) { $this->id = $id; $this->name = $name; $this->...


1

A modern (PHP7+) solution in case you want a default value when either the element is not set or the value is null. $my_variable = $array['possible_key'] ?? 'my default value';


2

Initial Feedback The code looks quite modern - with return types declared, chaining supported, etc. The code is somewhat in-line with PSR-12 and thus is quite readable, though a few rules aren't followed - e.g. 4.4 Methods and Functions Method and function names MUST NOT be declared with space after the method name. The opening brace MUST go on its own ...


3

As long as this input array is static in its depth and structure, the best and fastest technique (due to lowest overhead) is to use old-school loops. Beyond that, I would: Not declare variable that will not be used Use square brace pushing syntax Avoid excessively wide lines of code which demand the developer to horizontally scroll Use words as variable ...


2

The idea is very good and the structure is correct. Do not listen to anyone who would say that a crud method should be implemented in the controller. All the controller's job is to get the input and to call the CRUD method from the model. So in this regard your code is all right. However, your model code suffers from the opposite mix-up: it shouldn't format ...


2

IF logic All encompassing ifs Right off the bat you have two levels of nested if statements before we get to the main body of the code... if (!isset($_SESSION['id'])) { if (isset($_COOKIE['info'])) { // Main code... } // Additional code... } ...which is unnecessary; you could just as easily merge them into one statement: if (!isset($...


2

I'm aware that the OP appears to no longer be a part of the site. However, I thought that this was an interesting topic that may have relevance to future visitors as well General HTML & PHP The first thing I suggest is that you move all of your HTML code blocks to the top of your document. That way they're out of the way and don't interfere with your ...


3

OutputResponse Quotation marks This is hugely pedantic: your 405 header response uses single quotes where as all of the others use double quotes. It makes no difference (in fact single quotes are slightly faster for plain text in PHP), however, it's good practice to stick to one method especially when setting near identical strings. Type hints & ...


1

I'm trying to make a simple code to upload an image directly from a url. You mean download. How I can make it more safe? Is there any reason why you want to retain the original file name, after applying the basename function ? This should protect against basic path traversal attempts like ../../ but you could as well rename the file altogether. If you are ...


3

if (empty($_POST[$item]) || !array_key_exists($item, $_POST)) { is literally checking if $_POST[$item] is not declared or is falsey THEN checking if $_POST[$item] is not declared. // Check for nulls or non existent payload The comment associated with the step indicates that you want to check if $_POST elements are not declared or are null. In this case, ...


2

An interesting approach. Being a dev, I never bothered myself with proper English syntax, using just implode and calling it a day. but yours looks definitely better. Regarding optimization, you may notice that a condition $missingCount === 2 is actually covered by else case, so it can be removed. Or so I think because I believe that the last comma is ...


3

Overview OOP vs Procedural mysqli Firstly, it's important to note that the difference between the two methods is the way in which you, as a developer, interact with them. They both use use OOP one just hides it behind a veil of procedural functions. In fact, you can mix and match: // OOP $query = $mysqli->query("SELECT * FROM table"); while( $...


1

Procedural Prepared Query Structure This is the typical layout of a prepared query, in a procedural setting: $sql = "..."; $query = mysqli_prepare($mysqli, $sql); mysqli_stmt_bind_param($query, "si", $stringValue, $integerValue); mysqli_stmt_execute($query); The key points are that you: Don't need to carry out your own custom ...


2

Note: this is intended to be a Community wiki since it was taken from a deleted answer by yannis; The suggestion to do this was here Using two message instances is fine as is most of your code. There is one major issue though, you don't validate the email, you just check if the $_POST variable was set and is not empty but you don't check if the email format ...


4

In addition to the magnificent answer from KIKO Software, a few pointers... Getting IP First of all there is at least one issue connected to security; namely the findIP() function. Many beginners are using a similar approach and feeling smart. In reality, this function makes your code less reliable and secure as having this very function actually allows the ...


5

Naming things I love that you take the time to name things properly. Names like create_db_linkage_instance(), endTheWebpage() immediately make clear what these functions do. It's not db_conn() or finish(). Of course there is the problem that you haven't stuck to one naming convenstion, choose either snake_case or or camelCase and stick to it. Yes, I know ...


Top 50 recent answers are included