2

New! Save questions or answers and organize your favorite content.
Learn more.

\$\begingroup\$

I have this code and this works fine but I think it's too lengthy. I want to know if there can be a less lengthy code. I've heard from people that lengthy code is often not considered good.

<html>
<head><title>Table</title></head> 
<body>
<form method = "Post" action = "Table.php" > 
Enter a number between 2 and 20 to see its tables: 
<input type = "text" name = "number" /> 
<input type = "hidden" value = "true" name = "checker" /> 
</form> 
<?PHP 
if ( isset($_POST['checker'])) { 
$number = $_POST['number']; 
switch ($number) { 
case 2: 
for ( $counter = 1 ; $counter <= 10 ; $counter++ ) {
$temp = $number * $counter ; 
echo "$number x $counter = $temp<br />"; 
} 
break;
case 3: 
for ( $counter = 1 ; $counter <= 10 ; $counter++ ) {
$temp = $number * $counter ; 
echo "$number x $counter = $temp<br />"; 
} 
break;
case 4: 
for ( $counter = 1 ; $counter <= 10 ; $counter++ ) {
$temp = $number * $counter ; 
echo "$number x $counter = $temp<br />"; 
} 
break;
case 5: 
for ( $counter = 1 ; $counter <= 10 ; $counter++ ) {
$temp = $number * $counter ; 
echo "$number x $counter = $temp<br />"; 
} 
break;
case 6: 
for ( $counter = 1 ; $counter <= 10 ; $counter++ ) {
$temp = $number * $counter ; 
echo "$number x $counter = $temp<br />"; 
} 
break;
case 7: 
for ( $counter = 1 ; $counter <= 10 ; $counter++ ) {
$temp = $number * $counter ; 
echo "$number x $counter = $temp<br />"; 
} 
break;
case 8: 
for ( $counter = 1 ; $counter <= 10 ; $counter++ ) {
$temp = $number * $counter ; 
echo "$number x $counter = $temp<br />"; 
} 
break;
case 9: 
for ( $counter = 1 ; $counter <= 10 ; $counter++ ) {
$temp = $number * $counter ; 
echo "$number x $counter = $temp<br />"; 
} 
break;
case 10: 
for ( $counter = 1 ; $counter <= 10 ; $counter++ ) {
$temp = $number * $counter ; 
echo "$number x $counter = $temp<br />"; 
} 
break;
default:
echo "Please enter a number between 2 and 10"; 
} 
} 
?> 
</body>
</html>
Improve this question
\$\endgroup\$
5
  • \$\begingroup\$ State only code-purpose in title \$\endgroup\$
    – Caridorc
    Sep 24, 2015 at 18:03
  • \$\begingroup\$ I'm new at stackoverflow so don't know how to ask a question. Sorry for the mistakes. \$\endgroup\$
    – user5237857
    Sep 24, 2015 at 18:04
  • 2
    \$\begingroup\$ Here, on CodeReview, we'd like you to state what your code is supposed to do, and to have a descriptive title, telling what your code does, not what you want reviewed :) \$\endgroup\$
    – IEatBagels
    Sep 24, 2015 at 18:07
  • \$\begingroup\$ I suggest Generating tables of numbers in PHP as the title \$\endgroup\$
    – Caridorc
    Sep 24, 2015 at 18:10
  • 1
    \$\begingroup\$ Welcome to Code Review! This question is incomplete. To help reviewers give you better answers, please add sufficient context to your question. The more you tell us about what your code does and what the purpose of doing that is, the easier it will be for reviewers to help you. See also this meta question. \$\endgroup\$
    – Malachi ♦
    Sep 24, 2015 at 18:11

1 Answer 1

Reset to default
2
\$\begingroup\$

Already gave in the wrong placed question, anyway here is my solution: codepad.viper-7.com/zTLliA

Copy&paste is never a good idea.

if (isset($_POST['number']) && is_numeric($_POST['number']) && (($_POST['number'] > 0) && ($_POST['number'] <= 20))) { 
    $number = $_POST['number'];
    for ( $counter = 1 ; $counter <=10; $counter++ ) {
        $temp = $number * $counter ;
        echo "$number x $counter = $temp<br />"; 
    }
} else {
    echo "Please enter a number between 1 and 20"; 
}

A single for is enough. In the if statement we check if the $_POST['number'] exists, if it's actually a number and if it is between 1 and 20.

Improve this answer
\$\endgroup\$
3
  • \$\begingroup\$ Please include code in question and explain the improvements we mainly improve coders, code only incidentally. \$\endgroup\$
    – Caridorc
    Sep 24, 2015 at 19:13
  • \$\begingroup\$ @Caridorc I thought it was quite self explanating, by the way I have improved the answer. Hope it helps, have a nice day. \$\endgroup\$
    – phaberest
    Sep 24, 2015 at 19:29
  • 1
    \$\begingroup\$ A single for is enough. In the if statement we check if the $POST_['number'] exists, if it's actually a number and if it is between 1 and 20. <- that is an explanation +1 and welcome to Codereview \$\endgroup\$
    – Caridorc
    Sep 24, 2015 at 19:33

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.