patternjavaMinor

How is my program for computing the Hardy-Ramanujan number?

Submitted by: @import:stackexchange-codereview·Mar 10, 2026·

Viewed 0 times

thenumberprogramramanujanhardyforhowcomputing

Problem

I'm just wanting feedback on the efficiency, design and layout of this code if possible please. I suspect it's weird having a constructor with nothing in. I'd be grateful if it could be explained how to modify the code to fit in with conventions, especially in this respect, but also in others.

```
package cubes;

import java.util.ArrayList;

public class Cubes {

ArrayList sum = new ArrayList();
ArrayList x = new ArrayList();
ArrayList y = new ArrayList();
boolean found = false;
int cSum = 0, a = 0 , b = 0, c = 0, d = 0, k, floor;
Integer cubeSum, X, Y, answer;
double K, J, cs;
int iterations;

public static void main(String[] args) {

Cubes c = new Cubes();
c.find();
}

public Cubes()
{

}

private void find()
{
int i = 3;

K = (double)2;
J = (double)1;
cs = Math.pow(K,3.0)+Math.pow(J,3.0);
cubeSum = new Integer((int) cs);

sum.add(cubeSum);
x.add(1);
y.add(2);

i = 4;

while(found!=true)
{
floor = (int)Math.floor(i/2);

for(int j = 1; j < floor+1; j++)
{
k=i-j;
K = (double)k;
J = (double)j;
cs = Math.pow(K,3.0)+Math.pow(J,3.0);
cubeSum = new Integer((int) cs);
X = new Integer((int) j);
Y = new Integer((int) k);

for (int l = 0; l < sum.size(); l++)
{
a = (int)x.get(l);
b = (int)y.get(l);
if(cubeSum.compareTo(sum.get(l))==0)
{
if (a != j && a != k && b != j && b != k)
{
c=j;
d=k;
System.out.println("The first positive integer "
+ "expressible as the sum\nof 2 different positive"

Solution

Discrete Mathematics

Discrete calculations should not be done using floating-point math. You should be suspicious of any results contaminated by floating-point math. The way to compute the cube of an integer is n n n. For larger powers, do repeated squaring, or use BigInteger.pow() (which is slower, but immune to overflow).

Variable Usage

To be honest, I found your code extremely difficult to read, mainly due to the way you misuse variables.

Scope: You should declare and define variables as close as possible to the point of use. If a variable only applies within a loop, declare and define it inside the loop. If it's local to a function, define it as a local variable in the function. Instead, you've used a lot of instance variables, which, for all practical purposes, are "global" (since you only have one class). As you really only have one function, all of the variables should have been local!

Proliferation: You've declared 19 instance variables. Surely fewer variables would suffice? A human brain can keep track of about 7 items at a time. You have cSum (an int), cubeSum (an Integer), and cs (a double) — which raises the question, how are they related? (Answer: cSum is never used, and cs is just a temporary variable that shouldn't have been needed at all.) Also, answer and iterations are never used. You've got so many variables that you couldn't keep track of the dead ones to prune.

Cryptic naming: Without studying the code, can you remember what x, y, k, X, Y, K, J, and floor mean? If not, then your variables are named too cryptically.

Unconventional naming: By connotation, I would expect x to represent a floating-point number, not a list of Integers. Then, you've also got X, which is a boxed version of j.

Misuse for flow control: To terminate the loop, why do you set a variable and test for found != true elsewhere? Just return after printing the result — you're done!

Language Disfluency

If you familiarize yourself with the Java language a bit more, you can express yourself less clumsily.

Prefer primitives: You should always prefer the primitives (e.g. int) to their boxed counterparts (Integer). Code written using primitives reads better and performs better. The boxed types exist mainly so that you can store them in collections such as ArrayList. Even then, the compiler will automatically box and unbox the values for you, so you don't have to say new Integer(n).

Type Promotion: The compiler will automatically perform widening conversions, such as promoting int to double, so casting is unnecessary in those cases.

Integer division: When dividing an integer by an integer, the result will also be integral (with truncation towards 0). When you do (int)Math.floor(i/2), you're just promoting i / 2 to a double argument, rounding it to the same double, and casting the result back to an int. It would have been sufficient to just do i / 2.

Time Out… Let's Rewrite!

Let's rewrite the code using the trivial simplifications mentioned so far.

import java.util.ArrayList;

public class Cubes {

    public static void main(String[] args) {
        new Cubes().find();
    }

    private void find()
    {
        ArrayList sum = new ArrayList();
        ArrayList x = new ArrayList();
        ArrayList y = new ArrayList();
        int a, b, c, d;

        sum.add((2 * 2 * 2) + (1 * 1 * 1));
        x.add(1);
        y.add(2);

        int i = 4;

        while(true)
        {
            for(int j = 1; j < i/2 +1; j++)
            {
                int k=i-j;
                int cubeSum = (k * k * k) + (j * j * j);

                for (int l = 0; l < sum.size(); l++)
                {
                    a = (int)x.get(l);
                    b = (int)y.get(l);
                    if(cubeSum == sum.get(l))
                    {
                        if (a != j && a != k && b != j && b != k)
                        {
                            c=j;
                            d=k;
                            System.out.println("The first positive integer "
                            + "expressible as the sum\nof 2 different positive"
                            + " cubes in 2 different ways\nis "
                            + cubeSum + " = " +a+"^3 + "+b+"^3 = "
                            + c+"^3 + "+d+"^3");
                            return;
                        }
                    }
                }

                sum.add(cubeSum);
                x.add(j);
                y.add(k);
            }
            i++;
        }
    }
}

I find the rewritten version slightly more readable than the original, though I still can't really understand it.

Now, back to our regularly scheduled programming…

Algorithm

Magic numbers: You start with x.add(1), y.add(2), and i = 4. Where did those magic numbers come from? They're not naturally part of the problem, and you didn't e

Code Snippets

import java.util.ArrayList;

public class Cubes {

    public static void main(String[] args) {
        new Cubes().find();
    }

    private void find()
    {
        ArrayList<Integer> sum = new ArrayList<Integer>();
        ArrayList<Integer> x = new ArrayList<Integer>();
        ArrayList<Integer> y = new ArrayList<Integer>();
        int a, b, c, d;

        sum.add((2 * 2 * 2) + (1 * 1 * 1));
        x.add(1);
        y.add(2);

        int i = 4;

        while(true)
        {
            for(int j = 1; j < i/2 +1; j++)
            {
                int k=i-j;
                int cubeSum = (k * k * k) + (j * j * j);

                for (int l = 0; l < sum.size(); l++)
                {
                    a = (int)x.get(l);
                    b = (int)y.get(l);
                    if(cubeSum == sum.get(l))
                    {
                        if (a != j && a != k && b != j && b != k)
                        {
                            c=j;
                            d=k;
                            System.out.println("The first positive integer "
                            + "expressible as the sum\nof 2 different positive"
                            + " cubes in 2 different ways\nis "
                            + cubeSum + " = " +a+"^3 + "+b+"^3 = "
                            + c+"^3 + "+d+"^3");
                            return;
                        }
                    }
                }

                sum.add(cubeSum);
                x.add(j);
                y.add(k);
            }
            i++;
        }
    }
}

import java.util.Iterator;
import java.util.SortedSet;
import java.util.TreeSet;

//////////////////////////////////////////////////////////////////////

/**
 * An iterator of positive perfect cubes, starting from 1, and ignoring
 * overflow.
 */
class Cubes implements Iterator<Long> {
    /**
     * An "infinite" set of all perfect cubes.
     */
    private static SortedSet<Long> allCubes = new TreeSet<Long>();

    /**
     * Helper to generate more members of allCubes as needed.
     */
    private static Cubes allIter = new Cubes();
    static {
        allCubes.add(allIter.next());
    }

    public static boolean isCube(long num) {
        while (num > allCubes.last()) {
            allCubes.add(allIter.next());
        }
        return allCubes.contains(num);
    }

    public static int cubeRoot(long num) {
        while (num > allCubes.last()) {
            allCubes.add(allIter.next());
        }
        return 1 + allCubes.headSet(num).size();
    }

    public static final long cube(int num) {
        return (long)num * num * num;
    }


    private int i = 1;

    @Override
    public boolean hasNext() {
        return true;
    }

    @Override
    public void remove() {
        throw new UnsupportedOperationException();
    }

    @Override
    public Long next() {
        return cube(this.i++);
    }

    public Cubes dup() {
        Cubes moreCubes = new Cubes();
        moreCubes.i = this.i;
        return moreCubes;
    }
}

//////////////////////////////////////////////////////////////////////

public class HardyRamanujan implements Iterator<Long> {

    private long n;
    private long[] breakdown;

    public HardyRamanujan() {
        this.n = 1;
    }

    @Override
    public boolean hasNext() {
        // Ignores overflow
        return true;
    }

    @Override
    public void remove() {
        throw new UnsupportedOperationException();
    }

    @Override
    public Long next() {
        while (null == (this.breakdown = hardyRamanujanBreakdown(this.n))) {
            this.n++;
        }
        return this.n++;
    }

    /**
     * Returns the breakdown of the current Hardy-Ramanujan number n as an
     * array of four perfect cubes [a, b, c, d].  n = a + b = c + d.
     * a < c <= n/2 <= d < b.  Returns null if next() has never been called.
     */
    public long[] breakdown() {
        return this.breakdown;
    }

    private static long[] hardyRamanujanBreakdown(long n) {
        long a, b, c, d;

        // Exists n == a + b, where a and b are cubes?
        Cubes cubes = new Cubes();
        while ((a = cubes.next()) < n / 2) {
            if (!Cubes.isCube(b = n - a)) {
                continue;
            }

            // If so, exists n == c + d, where c and d are cubes, and c > a ?
            Cubes moreCubes = cubes.dup();
            while ((c = moreCubes.next()) <= n / 2) {
                if (!Cubes.isCube(d = n - c)) {
                    continue;
                }
                return new lon

Context

StackExchange Code Review Q#34009, answer score: 6

Revisions (0)

No revisions yet.