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. ...
8
I'm going to go through a few main points here about your code and at the bottom I'll have a "what I would change" section with inline comments. Do not be overwhelmed by the size of this book, I prefer to be on the side of too much information than too little.
- Cache your selectors: Probably the most important thing you can do for your code now. As a rule ...
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 a class ...
5
Here are some tips for the JavaScript side:
1) Use $ instead of jQuery.
2) Use constant variables in place of numeric literals.
Old Code:
if ((e.ctrlKey || e.metaKey) && e.which == 81) {
New Code:
var CHAR_CODE_q = 81;
if ((e.ctrlKey || e.metaKey) && e.which == CHAR_CODE_q ) {
3) The id attribute value must be unique for all elements ...
5
Well, I can't help you with the WordPress specifics, or the jquery, which means not much at all really. But here are a few things that might help you with your PHP.
Foremost, I would definitely consider separating your HTML from your logic. Or at the very least separate your escapes from PHP so that your PHP does not run into your HTML. It makes for much ...
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 />');
...
4
One simple thing you could do is to use jQuery as $ inside your ready function.
Since the ready function takes a parameter to jQuery, you could capture that as $ for the duration of the ready function:
jQuery(document).ready(function($){
if ( $(window).width() < 960) {
...
That would likely help improve the readability of the code.
For more info ...
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
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
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
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 also “...
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 ...
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
Your code can be simplified a bit.
The parantheses on the right side is not needed. Instead of
$name = (get_field('name'));
you can just say
$name = get_field('name');
However, since you are outputting the values directly, I'd use another approach for preparation. Collect all output strings in an array:
$result = array();
...
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....
Only top voted, non community-wiki answers of a minimum length are eligible
