I did the FizzBuzz challenge in PHP. I am very new to PHP, how can I improve my code?

What you wrote works great! Good job! One of the next big things you'll start to learn is how to think a little more abstractly. A big concept you could improve on is DRY (don't repeat yourself). Keep in mind, the "improvements" in this case are not really going to make a difference in performance or anything, but it helps think differently and approach solutions from more angles.

Small note first - being strict with comparison will help you be more thoughtful of the code you're writing. I'd say in almost every case strict comparison is a better choice than loose. So === instead of ==

You've got four conditions that can be met. In each one, you're appending . ' ' - Instead of using continue to exit out of the loop, you could just let it reach the end of the conditions and then echo the space, meaning you only write echo ' '; once.

Look at your first two conditions; they both have $number % $v1 == 0, and in both of those cases, Fizz gets printed out. Similar with the second and third cases, $number % $v2 == 0 is in both, and Buzz is printed in both of those. Is there a way you could write this in two conditions instead of 3?

Again, these aren't really "improvements" as much as they are ways to think about the problem in different ways to come up with many solutions, so you can pick the best one.

When iterating incrementally, use for. In general, range() is almost always the wrong solution to any problem. Creating an array N elements long just to iterate from 1 to N is needlessly wasteful.

$v1 & $v2 should be constants not variables, as they contain magic values that never change. You should also use more descriptive names for variables and constants alike (e.g. maybe FIZZ and BUZZ in this case).

Runtime concatenation of string constants (i.e. "Fizz".' ') is rather pointless and ugly. I believe the engine will optimize this away, but still - not very readable. Either combine them ('Fizz ') or turn one or both into identifiers.

Your continue statements don't serve any purpose, as execution drops to the end of the loop in all cases and there is no other code to skip.

Get into the good habit of always using ===, and only == when absolutely necessary (which is virtually, if not literally never).

Keep your expression evaluations DRY and readable when you find yourself in the situation of needing to repeat them (see below)

And last and most definitely least, PSR-2 (or, better yet, 12) compliance would be nice, though if this code is written in a different standard that your team has settled on, that's perfectly fine, too.

the continues are pointless since nothing else would happen anyway without them. other than that the only suggestion i have is to use 3v4l instead of whatever that sandbox thing is. 3v4l runs your code in all versions of php, allows you title your test, and looks a hell of a lot nicer.

Everyone else has pointed out lots of improvements which I will not re-iterate. One that I have not seen anyone else mention is that imo the final else statement is redundant.

} elseif ($number % $v2 == 0) {
echo "Buzz".' ';
}
echo $number.' ';

To else or not to else is a bit of a debate so google it. Im in the camp of not else as it reduces code, improves readability and clears up any confusion as to its intended use.

Another is mostly to get you thinking about re-usability. You have in your code the same operation twice $number % $v1 == 0 and $number % $v2 == 0, you could assign them to a variable so you only need to calculate it once(as noted by another commenter) or you could implement some re-usable functions.

You could improve the factoring in your conditions. So instead of having if (($number % $v1 == 0) && ($number % $v2 == 0)) { you could create intermediate variables like $numberMultipleOf3 = $number % 3 == 0; and $numberMultipleOf5 = $number % 5 == 0;. In the end your condition can reuse those variable, and are more human readable: if( $numberMultipleOf3 && $numberMultipleOf5 ).

You also might want to speed up the interpretation of your code by manually forcing to not do casts over your numbers using triple equal: $numberMultipleOf3 = $number % 3 === 0;.

Another factoring would consist on creating a variable to hold the message. If you pay attention, you keep appending a . ' '; space to your strings and call for continue multiple times. Why not do like this: $message = 'Fizz'; or $message = $number;, and right before the loop finishes, echo $message . ' ';. If tomorrow you want to print HTML instead of plain text in the console, you will update only one line instead of severals.

Also, always good to put your functions calls outside loop, to help PHP understand it should not reevaluate the range(1, 100, 1) constantly (I guess it guess it should not anyway, but the guess might consume resources). Anyway, good practice would be to make another variable, $numbers = range(1, 100, 1); and keep your loop readable foreach( $numbers as $number ).

I’m on mobile so this isn’t going to be beautifully formatted but, imo the best way to do this is to start with a variable like $msg and concatenate with .=.

Also, ignore this if it’s excessive but I love optimization, you could create arrays where you store the factor you want to check by and it’s correlating key word. So $fizz = [3, “Fizz”] and $buzz = [5, “Buzz”].

For example:

I would then make a for loop to iterate over a set number with a variable $i, because convention. (Also define $msg = “” at the start of the for loop)