Hot answers tagged

8

Application Architecture The architecture of the application is divided into models and views to separate the data and business model from the user interface. Within the models there is an additional division, there are models that represent the database tables and there are models that represent a row of data within each database table. You missed ...


6

Problem Statement Tiered Architecture This database is the first part of the project, I couldn’t really create a friendly user interface until the database is working. Due to feature creep this database can now handle other kinds of books besides science fiction and fantasy. The most important step when taking on a project as you have, is deciding ...


6

I won't read this code beginning to end. Some basic advice: Namespaces ExperimentSimpleBkLibInvTool.ModelInMVC.DictionaryTabelBaseModel doesn't follow naming conventions. Naming convention is Company.App.Module. It should be something like: Pacmaninbw.PersonalLibrary.Model. Naming Don't tack on crap affixes to the names. ExperimentSimple... It's ok ...


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 $...


6

No, don't do that. You seem to be filtering and escaping values out of paranoia rather than understanding what exactly would lead to a vulnerability. As a result, you are corrupting your data. A well designed application should use the database to store the value that the user typed into the textarea, not some mangled representation of it. If you mangle the ...


5

The description of the problem in the post says that the case to be detected is redeeming "over 3 time" but what the code actually detects is 3 times or more. Which is right? The description of the problem in the post says that the case to be detected is redeeming: on same stamp card within 10 mins But what the code actually detects is redeeming: on ...


5

cymysql.connect is a context manager and so you should use it in a with statement. conn.cursor isn't a context manager. Which is very fishy, even more so that the original version of cymysql, pymysql, is. Seperate SQL interactions from UI interactions. This is as multiple parts of the UI may need to use the same SQL interactions, however since they're ...


5

Funny coincidence, I just created the solution for your problem (but in PHP). The common thing here is the one-time use of a prepared statement, the difference is in the query string and arguments, so what you need is a function that accepts a query string, an argument determining what execution function to use (executeQuery() or executeUpdate()) and the ...


5

Disclaimer: there will be a lot of links to my own site because I am helping people with PHP for 20+ years and got an obsession with writing articles about most common issues. I don't know much about restful stuff or whether it does matter, but there are many other areas of improvement. Let's review some of them The selectData() function First of all, ...


4

You've used a single class to encapsulate virtual all the behavior of Mad Libs. Although this code is syntactically correct, it is clear you started with existing code and just poured most of it into a class. That's not the best approach. Your code is also extremely inflexible. It cannot be reused for any other Mad Lib story, and that would be the whole ...


4

The fact that mysqli has nothing to do with PDO aside, there are several things about your code at whole: this "class Database" from some article is a dummy. It is absolutely useless and there is not a single reason to prefer it over original PDO. Let alone some flaws in the code inside. under no circumstances you should create a new database connection in ...


4

This code is safe from the SQL injection standpoint but it is likely prone to XSS because of the untreated output, and also to possibly leak the sensitive information due to the error message unconditionally spat out. Regarding other improvements I would suggest a more robust connection code and to avoid bindParam calls thanks to disabled emulation mode. ...


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 ...


4

You must never store passwords as plain text in the database. Read an article about password hashing to avoid this mistake in the future. Make sure that you have a unique index on the username column. Otherwise it will be possible to create several users with the same username, and with equal or differing passwords.


4

By default you will have a numeric userId as well. Using the numeric userId rather than the username will probably perform better and reduce the size of the other tables. The userId will be a foreign key in 3 of the 4 tables and you need to consider the restraints that apply in each of the tables. Naming a picture in the user's profile pictureurl could be ...


3

You are using a transaction per insert statement, that will basically do nothing as a single statement on it's own is effectively a transaction. You want to perform the loop within a single transaction so that the disk is written just the once, rather than for every insert. I'd suggest changing :- for (int i = 0; i < ...


3

You use a date format function in the WHERE clause. This function then means that the query cannot use an index on the date column. Removing the date format function in the WHERE clause will improve the performance of the query.


3

Neat idea! Some suggestions: __exit__ should try to commit or roll back based on whether there is an unhandled exception, should then close the cursor, and should finally unconditionally close the connection. This is one of the main reasons for using a context manager - it is expected to always clean up after itself, leaving the relevant parts of the system ...


3

I don't know why you call keys of the $_POST Array "static" (and what does it mean at all) but the article you linked to (which I wrote) explicitly demonstrates that $_POST Array keys are unsafe and using them in the query untreated makes your code wide open to SQL injection. And no, your input_behandlung_sessions() is not the proper treatment. It makes ...


3

In your INSERT script: Use your config.php file everywhere instead of hardcoding your connection credentials. You should be checking that the POST elements actually exist before trying to access their values as a matter of best practice. I'd probably use a null coalescing operator or perhaps one giant isset() conditional (isset() can handle multiple ...


3

Lots of little pieces of advice: Tab the lines of code inside of your loops and functions (etc) for improved readability. Only declare the $key variable in your foreach() loop if you intend to use it. Avoid performing iterated trips to the database. Furthermore, always endeavor to minimize total trips to the database unless you have a data volume issue to ...


3

I don't know anything about mysql, so I'm ignoring that part of your question. For SQL Server, if you're trying to make an insert go faster you're going to want to: Do it in bulk Do it in parallel Make it minimally logged Do it in batches There are some things you can do that will handle all of this for you, which I list below, otherwise you'll have to ...


3

This looks like an interesting project. It's been a while but I think I'll try answering it. Keep up the good work. class DatabaseCommit(): No need for parentheses here. Get rid of it unless you are extending from some class. DataBaseCommit is not a good name. Maybe Database ? Why are you reading configs in this class? def read_config(section, filename='...


3

A couple possible bugs: When a user registers, there's text saying that they get $1000 to start. I don't see where this money comes from in the code. In display_info(), if player_stands is True, I think it could be possible to display both "Busted! You lost!" and ""Dealer busted! You won" I don't see the point of passing still_playing to display_info. If ...


3

Don't materialize generators unless you have to These: non_aces = [c for c in person if c != 'A'] aces = [c for c in person if c == 'A'] take up memory, however inconsequential. Since you only iterate through them once, change the [] brackets to () parens to leave it as a generator. Choose a quote standard You have both single and double: print("[2]...


3

If you allow users to pass in arguments that are column names (for example array keys that you turn into column names). Then the keys could be modified to allow SQL Injection. It's not clear in the question if you plan to do this. If you just want to have dynamic arguments (with known columns) then preparing the query should suffice as all the user data ...


3

DB Design Your tables are not correctly normalized: codes.description depends on codes.code → requires a table with codes.code as primary key You are also missing constraints: ISIN.id primary key ISIN.name unique key additionalCredit.id, additionalCredit.code primary key codes.codeId, codes.code primary key Query Optimization mysql does not come with a ...


3

Naming Try not to use generic names like $file. Make your code read easy for others. Say $files_to_backup for example. Check for errors when it matters If backup is important: check that the backups succeeded: if cp "${files[@]}"${war}/mediawiki_specific_backups";then rm -rf "${war}/${domain}" mkdir "${war}/${domain}" ... ... else #...


2

Aside from the excellent review from KIKO Software, I think it would be beneficial for you to understand some basic concepts, so here you are: Your sql queries are 100% safe because you are using prepared statements for the data literals. I don't really get what is your concern about mysqli_real_escape_string but be advised that this function has nothing to ...


2

Coding well is hard, but reading code should be easy After reading through your code I think it is reasonable secure. You protect yourself against SQL-injection and you use password_hash(). All good things. However this code only provides the most basic security, and that not very well. Sorry, I don't mean to be rude. There is one thing, I worry about, ...


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