FizzBuzz-like Assignment

I have an assignment for my Java programming class that is kind of an expanded FizzBuzz-like question:



-
Write a program that iterates through numbers from 0 to 113 using a loop. Print the numbers, one number per line. As you print each
number, say x, also print the following when appropriate, separated by
commas:

• If the number is odd, print “x is odd”

• If the number is divisible by 5, print “hi five”

• If the total of a number (x) and its subsequent number (x+1) is a value divisible by 7, print “wow”

• If the number is prime, print “prime”.

My main concerns are:

• Am I interpreting the condition for part 3 (isWow) correctly?

• Is there a better way of organizing the function that doesn't require a cascade of if statements?

• For an acedemic context, are the comments appropriate?

• Anything else that could be improved upon, including style.

```
package comp268.q9;

import java.util.ArrayList;

public class Number {

//I thought I might as well generalize the pattern.
public static boolean xIsDivisibleByY(int x, int y) {
if (y == 0) return false;
return x % y == 0;
}

public static boolean isDivisibleBy5(int n) {
return xIsDivisibleByY(n, 5);
}

public static boolean isDivisibleBy7(int n) {
return xIsDivisibleByY(n, 7);
}

public static boolean isOdd(int n) {
return !xIsDivisibleByY(n, 2);
}

public static boolean isWow(int n) {
return isDivisibleBy7(n + n + 1);
}

//A very simple prime checker.
//The only optimization I'm using is that I'm only checking factors up to
// the sqrt of x, since past there we're finding the "mirror" of factors
// we've already found.
public static boolean isPrime(int n) {
//Numbers iterate(int start, int end, boolean printDirectly) {
ArrayList allMessages = new ArrayList();

for (int n = start; n iterate() {
return iterate(0, 113, true);
}

Am I interpreting the condition for part 3 (isWow) correctly?

I think so.


Is there a better way of organizing the function that doesn't require a cascade of if statements?

There is a way, but I'm not sure it's worth it.
For example,
you could:

• Create an interface with a single method that takes an integer and returns a string

• Create classes that implement this interface, each class applying one of the prescribed check, and returning a non-empty string when applicable

• Put the classes in a list

• As you iterate over the numbers, replace the cascade of if statements with a loop over the list of the implementing classes

But I don't think it's worth it.
The problem is not interesting enough to overengineer it.


For an acedemic context, are the comments appropriate?

Honestly, your code is so easy to read that I completely disregarded the comments, they were invisible to me until now.
Now that I read them,
I see that some are too chatty,
most are unnecessary,
and some are completely incorrect.
("I could just mutate the passed String." -- no you cannot mutate Strings in Java.)
Overall, the comments hurt you more than help you.
You don't need them, delete them.


Anything else that could be improved upon, including style.

-
Refer to types using interfaces instead of implementations: use List instead of ArrayList in type declarations

-
When doing a lot of string concatenations, consider using a StringBuilder

-
Most of your names are descriptive, but some are really not good:

• addToList sounds like adding to a list, but concats the params and returns a String

• iterate : the name suggests a void method iterating over a collection, but that's not what's happening here. The term "iterate" is typically used in the iterator pattern, and it's somewhat inappropriate here, especially because the method returns a list. Perhaps checkNumbersInRange would have been better.

• xIsDivisibleByY feels really awkward. I think divisibleBy(x, y) would be just fine

-
iterate does too many things: it returns a list, and sometimes it also prints. The printing doesn't belong there.

-
It would be good to extract the actions in the main loop body to a separate method (with the cascade of if statements)

StackExchange Code Review Q#91957, answer score: 2