12
One of the nested loops can be eliminated, by joining these two queries:
$query = "
SELECT *
FROM imagemap
WHERE exercise = '".$data['ExerciseID']."'
LIMIT 1
";
$result = $wpdb->get_results($query);
foreach($result as $row) {
echo '<div class="imageGridImages imageGridImagesLarge">';
global $wpdb;
$exerciseTitleQuery = "
SELECT ...
11
This is going to be a bit of a drive-by code review due to time limitations and my limited php knowledge, but hopefully I can add some value here. Here are some high-level things I've noticed about this code:
PARAMETERIZE YOUR QUERIES!. This is the most common security hole found on the web, so make sure you understand the problem and how to combat it.
Use ...
8
The biggest performance issue is probably here:
foreach( $order_items as $order_item ) {
$item_meta = $wpdb->get_results("
SELECT *
FROM {$wpdb->prefix}woocommerce_order_itemmeta
WHERE order_item_id = " . $order_item->order_item_id . "
");
// ...
}
If you have 1000 order items, you will run a 1000 queries. ...
7
#three-column input[type="text"], input[type="email"], input[type="tel"]
I'm pretty sure should be
#three-column input[type="text"], #three-column input[type="email"], #three-column input[type="tel"]
because your probably mean all inputs in #three-columns. The selectors after the commas don't get the #three-columns from the first one.
On the other hand,
...
7
The first small thing that pops out is array( &$this, 'method_name' ). That's PHP 4 and you can totally drop the & (WordPress requires PHP 5.2.4+).
The second is that if your going to use localization, better to put all in place throughout your code, ie: $translated = __( 'Hello World!', 'mytextdomain' );.
And finally, and maybe that's micro-...
6
Yes, it's extremely inefficient, because in every iteration you count the number of words twice: once in count_words, and then wp_trim_words internally has to count the words again.
Another problem is that the method won't work for content that is many words larger than the target limit length. For example with a content of 4000 chars and a limit of 100, ...
6
Separation of concern
You mention "separation of concern" in your question. It's fantastic that you actually know about it, and are trying to adopt good practices whilst learning about OO. However, allow me to be a pedantic tw*t for a second, and stress that SoC is not the same as SRP (Single Responsibility Principle).
It's the SRP that states that ...
5
Sometimes the nested loops cannot be avoided in certain areas, however the purpose is not just to have optimized code, but also more readable code.
The code needs to be separated as units or each logic in a function. So there should be one function which does only one foreach and the arguments comes from another function which calls this, should be calling ...
5
GET and setters
One of the points of @EliasVanOotegemto was that your class expects the $_GET variable to be set, which is not ideal because it makes your class very static and hard to reuse. What if you at some point do not store this data in GET, but in POST? Or in a session? You cannot use your class in that case.
That is why @EliasVanOotegemto ...
5
The logic for getting 100 posts is duplicated for the case of the initial load and subsequent loads with an offset. It would be better to refactor in a way to eliminate the duplicated logic:
params = {'number': 100}
offset = 0
while True:
response = requests.get(url, params=params).json()
found = response['found']
posts = response['posts'] ...
5
I would agree with discussion in comments that this should be looked at as a stop-gap solution only, as the real answer here is to change the HTML source, or better yet, the CSS styling for the desired h2's such that you don't need these span's at all.
If you truly want to wrap every h2 like this, why is this not just:
$('h2').wrapInner('<span />');
...
5
As mentioned by cas in the discussion, you can simplify your loops:
for dir in /var/www/html/*/; do
if pushd "$dir"; then
/usr/local/bin/wp plugin update --all --allow-root &&
/usr/local/bin/wp core update --allow-root &&
/usr/local/bin/wp theme update --all --allow-root
popd
fi
done
You can ...
4
your first function should use a switch statement instead of a complex if then statement.
foreach ( $this->pages as $page )
{
if ($page['type']=='menu')
{
add_menu_page(
__( $page['title'] ),
__( $page['title'] ),
'administrator',
$page['id'],
...
4
I'm just going to write some comments from the top down.
First of all I would like to point you to PSR, and especially PSR-2, since following a well known coding style will help other developers read your code. But please note that PSR-2 is nowhere near WordPress's coding standards (For a more entertaining read I also recommend the Linux kernel coding style,...
4
The code seems pretty solid. I'll make some small observations:
I'd remove add_action( 'after_setup_theme', 'remove_core_updates' ); and add a call to the function remove_core_updates() at the end of beeFramework_setup().
The use of anonymous functions, create_function, prevents something like:
# In your functions.php file
add_action( 'init', '...
4
First off, your markup is invalid. You have a paragraph containing block elements (div, other paragraphs), when they are only allowed to contain inline elements.
The rest of your markup is less than optimal, to be polite. You have so many empty elements being used to simulate whitespace when there's absolutely no need.
There are no labels used here at ...
4
Welcome to the world of OOP. I think its awesome you are trying something new and pushing yourself, keep going! Before you read on keep in mind this may seem harsh. I have no intention of hurting you. In fact the opposite. By mentioning everything I see as an issue I want to help you. So without further ado, lets get to it then!
The main stuff
There are ...
4
In terms of security, you should be safe from SQL injection since you are using parameterized queries as recommended. That's true whether or not you validate the names using the regex. Do not perform any additional escaping — that would only mangle your data.
That regex is for enforcing your business rules (i.e. you want to reject names written in ...
4
Your code follows most of PEP8.
However:
Lines should be a maximum of 79 characters. The exception to this is comments, at 72.
Operators should have a space either side of them. 2 * 2. The exception to this is to show precedence: 2*2 + 2.
You should not have any spaces around the = when passing arguments:
response = requests.get(url, params= {
Your ...
4
This is a partial review.
$file_loc and $json_folder
Use $json_folder in the construction of $file_loc. If you decide to use a different folder, you need change two variables.
$file_loc = "{$json_folder}/twitter.json";
Naming of function
You named your function get_twitter_json()... which... does essentially nothing. It does not return json like the ...
4
Since the other answer and comments already pointed out the semantical optimizations, here's a rather syntactical one. The code does exactly the same as the one you have, but faster.
$(".bottomLine").each(function() {
$(this).insertAfter($(this).parent().find(".price"));
});
$(".prod-desc > h2, h2.review_text, .related > h2, .up-sells > h2")....
4
Inside of a function call, avoid echoing. By hardcoding echoes, you prevent the "silent" usage of the function. It may be necessary in the future to present the output in more than one format, so use a return inside the function declaration and perform the echo on the function call.
Pay close attention to psr-2's guidelines on control structures. They ...
4
Here is an improvement, but I would recommend a few additional changes as well.
<?php global $post; ?>
<div class="col kb_landing_left">
<?php
$post = get_post(56);
setup_postdata($post);
get_template_part('template-parts/content', 'page');
?>
</div>
<div class="col -sm-6">
<?php ...
3
Yes, this code does look like a performance disaster! I will try to review what I can, but without proper profiling and lack of data structures and code context, it's nearly impossible for someone besides you to streamline this code!
I'll work my way from top to bottom, although there are several things that are screaming to be reviewed!
I cleaned up your ...
3
Your code looks good, (although I did not review each particular) The only thing that stood out to me is that your files do not begin with a check to see if the file is being accessed directly. You should have a variable defined at the beginning of the theme that is checked for at the beginning of each other file, and if the variable is not defined, reject ...
3
There are several issues I have with your function. Some are about your function breaking the basics about what a function is/should do, others are perhaps a tad more personal (as in: a matter of personal preference).
Functions do stuff, without it impacting the rest of the code
It seems almost silly to explicitly state this, but let me explain what I mean....
3
I haven't really looked into your code that much, but the first thing I noticed is that you have a number of methods (like the constructor) that you expect to be an array.
That's fine, sometimes you want an array to be passed. But if you really need an argument to be an array, enforce it by using a type hint:
public function thisMethodRequiresAnArray(array $...
3
One thing that is quite important to really understand. It's something that is said all too often, but it's very true:
Premature optimzation is the root of all evil
By your own admission, you're using wordpress, and post some 30-odd lines of PHP code, asking us what you could do to improve performance. Well, I can tell you that some parts of your code ...
3
I would make a single function, since all your functions do the same thing. It just needs an extra argument.
/// one all-purpose function
function saveData ($user_ID, $data_field) {
$data = !empty($_POST[$data_field]) ? wc_clean($_POST[$data_field]) : '';
update_user_meta( $user_ID, $data_field, $data );
}
/// define actions
...
3
Your approach seems fine, assuming that the entries in $child_pages are the correct order. I mention that because you mentioned the ordering requirement in the description, but I don't see any supporting logic in the posted code.
If you need to sort the entries yourself, another solution can be sorting using usort() and a custom comparator function:
if the ...
Only top voted, non community-wiki answers of a minimum length are eligible