Return to Revisions

Code structure

That's a bit too much code for a single review. Given the task can be seamlessly separated into several modules, it will make it simpler to write, to read, to maintain and to review as well. For example, all the table creation code has no business with the main task of importing XML. The overall amount of code makes it harder to write a complete review, so I would suggest to post another question after taking into account all the suggestions given here. You see, a well-split code will produce a solid review and a solid code will produce a ragged review.

• Usually, the table creation is supplied in the form of an SQL file or a migration but not in the form of PHP code.
• the database connection code should be also moved into a separate file that just included in the main script
• many SQL queries can be put into functions as not to pollute the main operation loop. Like, the code below //Check if author already inserted into table should be just one line, $author_row = getAuthor($pdo, $book['author']); All those functions again can be put into the included file and offered for the separate review. Or at least as a separate block of code in the same review.

Logical consistency.

• It is possible that two authors can give their books the same name. It would make sense to check both the name and the author.
• there is some folder structure mentioned but no code provided to handle it. A built-in recursive directory iterator would do though
• you forgot the book update code. That's a direct result of your "wall of code" approach. With the main logic fits in one screen, it's easy to spot the mistake. While in such a wall of code it's easy to overlook one.

Formatting and readability

The overall formatting is good, your indentation is even and lines are reasonably long. Two suggestions:

1. Do not create a default indent. The baseline of PHP statements should be a the very left position.
2. Omit the closing PHP tag, it is forbidden by the standard
3. Using bindParam is a bit verbose. You can make it into one line, $stmt->execute([$last_author_id,$book['name']]);
4. The way you are using heredoc is way too verbose and doesn't actually use the benefits of heredoc. For my personal taste, heredoc is not needed here at all, and the entire assignment can be done in a single line using standard quotes: $sql = "SELECT * FROM authors WHERE name = ?"; - simple, compact and readable. But in case you prefer heredoc, at least move the closing tag to align with the opening tag, so there will be no extra spacing in the query

Error reporting

Using try catch around PDO connection is inconsistent and verbose. Given the error can occur on the every line, why only the PDO connection code gets such a special treatment? It just makes no sense. Besides, it's just convenient to see all errors on-screen dring development. You can (and should) configure the uniform error reporting for the entire PHP code. See the basic error reporting rules

Cache

You can avoid some roundtrips to database by storing known authors in the array like this $known_authors[$name] = $last_author_id; so next time you can try to get the id by simple $last_author_id = $known_authors[$name] ?? false; and only get to DB if it fails.

Other improvements.

1. Why not simplexml_load_file()?

2. The tableExists implementation is barbaric. There are more civilized ways to check the table existence than causing a deliberate error.

3. Using SELECT query to get the autogenerated id is WRONG on so many levels. Not only it's too verbose as compared to the proper way which is just $last_author_id = $pdo->lastInsertId(); but using SELECT query is guaranteed to give wrong results.

4. Come on, using for to iterate over arrays is prehistoric. I'd break my fingers trying to write

this

for($i = 0; $i < count($booksArr['book']); $i++) {
    echo $booksArr['book'][$i]['author'];
}

instead of just

foreach ($booksArr['book'] as $book) {
    echo $book['author'];
}