Since you're new, I'll do for you what I wish someone had done for me.
I've re-written your code to highlight some elements of simplification. If you'll bear with me, this will hopefully save you mountains of time and frustration. :-)
Like most beginners, you are using nested if statements because you have not yet learned to think like a professional programmer. You're mentally looping through the cells in your head and you're making decisions about them. Your code reflects that.
Lets look at what you did:
- You needed to fill up this block, so we randomly put an X here. That's somewhat OK, but there's a better way.
- You create a header row. also, not bad.
- You're making a decision on each cell based on its content. But this is where you start to make things complicated. Not only are you needlessly printing the tr bgcolor (doesn't matter when you change the cell color), but you'r making an either or decision with an else inside a loop. That's hard to read, hard to maintain, and if you continue to develop this way, other developers will want to strangle you.
- You have a secondary for loop to handle squares. This is wholly unnecessary.
But it's OK! You should see the trash I wrote when I was starting out!
Let's change the way you think to fix the code.
You probably did not start out by drawing your table on a piece of paper and coloring in your cells, but you should have. It helps you visuallize what you're about to do. I had the benefit of just running your code to see what you were doing. Quite honestly - I wasn't able to visualize it just looking at your code.
After I ran it, I saw you have three objectives:
- Show odds in grey (also the default color)
- Show evens in blue
- Show squares in white
Now, let's think about what actually has to go on here to make this happen.
- We need a header
- We need to loop through 1 to 12 twice to create the table.
- We need a header column so we can create that nice "x * y" lookup table
- We need to change the cell color based on its value.
So, let's use a couple of rules to force us to write cleaner, better code.
- Implement separation of concerns so that any given section of code is only doing one thing.
- Avoid nested loops and nested if's like the plague. Only use them when necessary. (We need a nested loop to do our mutiplication, so it's ok there).
- 'else' is never necessary.
- When making decisions, allow the decision to cascade down a decition tree until a matching condition is found.
- KISS
- Don't repeat yourself.
Here's the resulting code (note" I am using images because it will be easier for me to annotate the changes. The code is here in this gist for you.
The HTML
The HTML section is at the bottom of the code, and contains two simple PHP snippets to print the header and print the table.
Printing the header
Keeping with simplicity and separation of concerns, this does one thing: it prints the header. I also changed $l to $i since i is more synonymous with an index or pointer in programming.
And, I fixed a bug. You were using <12 instead of <=12 so your last column wasn't printing.
Printing the table
Again, separating out concerns, this function only wants to print the table. It doesn't care about the cells.
Here's what we're doing:
- $x represents the row number.
- Here, I open the row. Because each time $x increments, I should be starting a new row.
- $y represents the column. I need this here only because the column matters to the row. I am NOT concerned with printing the cell in this function.
- This function separates teh concern of dealing with the cell to another function aptly named print_cell.
- After I have printed all the cells for the row, I should close it out so I can start a new row, or finish out the table.
Printing the cells
Again - separation of concerns: I only want to deal with printing the cell. And, thus far, I don't care about colors!
- I need $x and $y, so I receive them here.
- I am using printf and a format string instead of concatenation. Anytime I see an abundance of concatenation, I know I am dealing with a novice programmer who doesn't know any better. You now know better. :-)
- This line determines the color, but you can see I refactored out the color logic into its own function because this function doesn't care about making color decisions - only about printing the cell.
- This line is a convenience so I can deal with $n instead of $x * $y. It fills up the symbol table uselessly, but sometimes you can just splurge on a little extra memory.
- This is a ternary operation, which is a basic shorthand for if...else. It doesn't violate the "else is never necessary" rule because it is limited to a single line and an "either or" condition, whereas "else" is a catch-all. It also avoids block if decisions. It's still clean code.
- Here, I am printing the cell using printf.
Choosing cell colors
First, there is no need to color the <tr> because we are choosing all the cell colors, which would just override it anyway. So we don't care about <tr>'s bgcolor.
Next, Please note that the numbers in the image below do not go from top to bottom like the others 1 is at the bottom for a reason!
This function is dedicated to choosing a cell color based on its content. We want it to execute from top to bottom, and return the color for the cell as soon as it gets a match. Therefore, we put the "special" colors towards the top of the function and default, non-special ones at the bottom.
- This is the default value. Cells are grey unless they are special. In order for a cell to be grey, they cannot match any of the rules above it.
- Our first column for each row is actually a header. In your original code, you were creating extra code to handle this, but in reality, it's just column 0. When $y == 0, make it blue.
- This is our first "special" condition for a cell color. If it's a square it's white (doesn't matter if it is even or odd).
- This is our next special condition, but not as special as squares. If it's even, it's blue.
What's very important about this type of structure is that it allows the processor to return a value from a function the instant it gets a result. Nested if's, on the other hand, have to exit if after if after if after if after else before they can exit the function. This doesn't matter in an application like what you've written, but at scale when every CPU cycle counts, that's important. Plus, this is DRASTICALLY easier to read than what you wrote with all the nested statements and concatenated strings.
Conclusion
Welcome to the programming club. I hope you found this useful and helpful. Keeping your code clean and following these pracitces (as well as knowing the PSRs, and how to refactor) will go a long way in making you an excellent addition to our community. (Not just SO, but the programming community in general).
Knowing design patterns is extremely helpful too.
Remember: "Always code as if the guy who ends up maintaining your code will be a violent psychopath who knows where you live." Because sometimes that violent psychopath will be you in 6 months when you're trying to figure out what the hell you did when you wrote "that code" six months ago.
Here's the re-written code
<?php
/**
* Prints the top row of the table.
* */
function print_header() {
echo '<th>X</th>';
$format = "<th bgcolor=#d3d3d3>%s</th>";
for( $i = 1; $i <=12; $i++) {
printf($format,$i);
}
}
/**
* Determines the color of the cell.
* */
function get_color($x, $y) {
//These are header rows / columns.
if($y == 0) return "#CAE3FF";
//If they are squared, x == y. This is the override.
if( $x == $y ) return "#ffffff";
//If they are even, return blue.
if( ($x * $y ) % 2 == 0 ) return "#CAE3FF";
//what's left over is either odd or weird. Return grey.
return "#d3d3d3";
}
/**
* Prints the cell
* */
function print_cell($x, $y) {
$format = "<td bgcolor = %s>%s</td>";
$color = get_color($x,$y);
$n = $x * $y;
$output = ( $n == 0 ? $x : $n);
printf($format,$color,$output);
}
/**
* Prints the rows of the table and holds the primary loop
* */
function print_table() {
for($x = 1; $x <=12; $x++) {
echo "<tr>";
for($y = 0; $y <=12; $y++) {
print_cell($x,$y);
}
echo "</tr>";
}
}
?>
<head>
<meta charset="utf-8">
<title>Multiplication Table</title>
<link href="style.css" rel="stylesheet" type="text/css">
</head>
<body>
<h1 align="center">Multiplication Table with a "forloop"</h1>
<table border ="1" align="center" width="80%">
<?php print_header() ?>
<?php print_table() ?>
</table>
</body>
</html>
