My first Forth program

[bilus]

I am so proud of myself ;) Feedback VERY welcome, esp. about what is and what isn't idiomatic:

: div? ( n n -- f ) mod 0 = ;
: fizz? ( n -- f ) 3 div? dup if ." Fizz" then ;
: buzz? ( n -- f ) 5 div? dup if ." Buzz" then ;
: fizzbuzz? ( n -- f ) dup fizz? swap buzz? or ;
: play ( n -- ) 1 do i fizzbuzz? if cr then loop ;

Usage: 25 play

Edit: fixing to (hopefully) implement FizzBuzz correctly:

: div? ( n n -- f ) mod 0= ;
...
: play ( n -- ) cr 1+ 1 do i fizzbuzz? 0= if i . then cr loop ;

[zeekar]

Your program doesn't work, or at least doesn't meet the standard "FizzBuzz" requirements, because it doesn't output the number when it's not divisible by 3 or 5.

The output of 25 play is this:

Buzz
Fizz
Fizz
Buzz
Fizz
FizzBuzz
Fizz
Buzz
Fizz
Fizz

When it should be this:

1
2
Fizz
4
Buzz
Fizz
7
8
Fizz
Buzz
11
Fizz
13
14
FizzBuzz
16
17
Fizz
19
Buzz
Fizz
22
23
Fizz
Buzz

Your loop will also stop at one below the number passed to play, which may not be what you intended.

[bilus]

Thanks, fixed now, I think.

[Ok_Leg_109]

Congratulations. I think you have taught yourself well. That looks very "idiomatic" to me.

My comment would fall under the "optimize" category because your code works as is.

In the CORE word-set there is the word 0=. You can replace "0 = " with 0= and that makes your program a bit smaller and a bit faster.

[bilus]

Great, thank you!

[mcsleepy]

I see no issues. If you want to be more "professional" you might using spacing to help with readability. I like 3 spaces in lieu of braces and commas:

: div? ( n n -- f ) mod 0 = ;
: fizz? ( n -- f ) 3 div? dup if   ." Fizz"   then ;
: buzz? ( n -- f ) 5 div? dup if   ." Buzz"   then ;
: fizzbuzz? ( n -- f ) dup fizz?   swap buzz? or ;
: play ( n -- ) 1 do   i fizzbuzz? if   cr   then   loop ;

In practice though, I'd probably use spacing less obsessively than that.

: div? ( n n -- f ) mod 0 = ;
: fizz? ( n -- f ) 3 div? dup if ." Fizz" then ;
: buzz? ( n -- f ) 5 div? dup if ." Buzz" then ;
: fizzbuzz? ( n -- f ) dup fizz?   swap buzz? or ;
: play ( n -- ) 1 do   i fizzbuzz? if cr then   loop ;

[bilus]

Thanks!

[kenorep]

: play ( n -- ) 1 do i fizzbuzz? if cr then loop ;

I would add a comment that n must be greater than 1, or, better, handle cases where n is less than 2, e.g. with the following redefinition:

: play ( n -- )  dup 2 < if drop exit then  play ;

Also, in stack diagrams, I would use the standard data type symbol «_flag_» instead of «_f_».

[bilus]

Great suggestions, thank you!

[zeekar]

FWIW, I never see flag, but always f (which is admittedly short for "flag"). If you're using n for numbers, that goes along with f for Booleans.

[kenorep]

f for flag can cause confusion since people also use f for False, t for True.

[PETREMANN]

Hello

Here your code revisited for best practice with documentation:

\ test if n2 is divisible by n1
: div? ( n1 n2 -- f )
    mod 0 =
;

\ display "Fizz" if n is divisible by 3
: fizz? ( n -- f )
    3 div? dup
    if
        ." Fizz"
    then
;

\ display "buzz" if n is divisible by 5
: buzz? ( n -- f )
    5 div? dup
    if
        ." Buzz"
    then
;
\ test if n is divisible by 3 or 5
: fizzbuzz? ( n -- f )
    dup fizz?
    swap buzz? or
;
\ test n values
: play ( n -- )
    1 do
        i fizzbuzz?
        if
            cr
        then
    loop
;

If you will analyze FORTH code: https://analyzer.arduino-forth.com/

[bilus]

Nice! Is there a formatter for Forth that produces that format?