Snippets

Jenny Rasmussen SkyVerge Code Review Snippets

Created by Jenny Rasmussen
skyverge-code-review-1.php
Raw 
 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
<?php
/**
 * This code retrieves course data from an external API and displays it in the user's
 * My Account area. A merchant has noticed that there's a delay when loading the page.
 * 
 * == What changes would you suggest to reduce or remove that delay? ==
 */
public function add_my_courses_section() {
	global $current_user;

	$api_user_id = get_user_meta( current_user->ID, '_external_api_user_id', true );

	if ( !$api_user_id ) {
		return;
	}

    /* Can you hit the get_api() once with the $api_user_id and then from that stored variable, check for the courses assigned and sso link(s)? I don't know exactly how this call is         set up, but I would prefer to do it this way
    
        $api_return = $this->get_api($api_user_id);
        
    	$courses  = $api_return->get_courses_assigned_to_user( $api_user_id );
	    $sso_link = $api_return->get_sso_link( $api_user_id );    
    
    Doing this would reduce the return time by half, because then you're only calling it once, and if you ever need to expand what you check and display, you already have the data       you need in $api_return
    */
    
	$courses  = $this->get_api()->get_courses_assigned_to_user( $api_user_id );
	$sso_link = $this->get_api()->get_sso_link( $api_user_id );

    // switched to echo from print, print has a return and a return isn't necessary here, might have a slight descrease in run time by not having to return
    // using the echo short notation <?= because even if it's the same performant runtime as writing echo, it's faster to type three characters
	?>
	<h2 style="margin-top: 40px;"><?= __( 'My Courses', 'text-domain' ); ?></h2>
	<table>
		<thead><tr>
			<th><?= __( 'Course Code', 'text-domain' ); ?></th>
			<th><?= __( 'Course Title', 'text-domain' ); ?></th>
			<th><?= __( 'Completion', 'text-domain' ); ?></th>
			<th><?= __( 'Date Completed', 'text-domain' ); ?></th>
		</tr></thead>
		<tbody>
		<?php
        
    // if you try and run the foreach without checking if there's anything in it, you'll show your users nasty errors, which would not only increase run time, but annoy users. 
    // also, is $courses an object or a multidimensional array? If its an object, you need to switch to -> notation instead of array notation
    if(!empty($courses) {
		foreach( $courses as $course ) :
			?><tr>
			<td><?= __( $course['Code'] ); ?></td>
			<td><?= __( $course['Name'] ); ?></td>
			<td><?= __( $course['PercentageComplete'] ); ?> &#37;</td>
			<td><?= __( $course['DateCompleted'] ); ?></td>
		<?php endforeach;
    }
		?>
		</tbody>
	</table>
	<p><a href="<?= $sso_link ?>" target="_blank" class="button <?= $_GET['active_course']; ?>"><?= __( 'Course Login', 'text-domain' ); ?></a></p>

	<?php
}
skyverge-code-review-2.js
Raw 
 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
<html>
<head>
<script>
setTimeout(
	function() {
		todos = document.getElementsByTagName( 'li' );
		i = 0;
        // should use === to prevent type coersion
		while ( typeof( todos.item( i ) ) === 'object' ) {
			todo = todos.item( i );
            // switched from evt to e, standard usage for an event triggered
			todo.addEventListener( 'click', function( e ) {
				e.target.style = 'text-decoration: line-through';
				alert( 'Item ' + ( i + 1 ) + ': "' + e.target.innerText + '" is done!' );
			} );
			i++;
		}
	},
	10
);
</script>
</head>
<body>
<ul id="todo-app">
	<li class="todo">Walk the dog</li>
	<li class="todo">Pay bills</li>
	<li class="todo">Make dinner</li>
	<li class="todo">Code for one hour</li>
</ul>
</body>
</html>

Comments (0)

HTTPS SSH

You can clone a snippet to your computer for local editing. Learn more.