Bad Java code

[subanark]

Someone asked me why their applet wouldn't work, stated that the error was the applet wasn't public. Aside from this one problem how many other problems can you state with this code? (reformatted for convience)

 

/
<applet code="appletframe" width=900 height=900>
</applet>
 /

import java.io.;
import java.awt.;
import java.awt.geom.;
import java.awt.geom.Arc2D.;
import javax.swing.*;
import java.lang.Double;

class mpplot

{
    static double average;

    static int numofmp = 0;
    static double array[] = new double[800000];

    public static void main(String args[]) throws IOException
    {
        String line;

        try
        {
            File file = new File(args[0]);
            final BufferedReader br = new BufferedReader(new FileReader(file));

            double z = 0;

            while ((line = br.readLine()) != null)
            {
                if (line == null)
                    break;
                try
                {
                    z = Double.parseDouble(line.trim());
                    array[numofmp] = z;
                    numofmp++;
                } catch (Exception e1)
                {
                    System.out.println(e1.getMessage());
                }

            }
        } catch (Exception e2)
        {
            System.out.println(e2.getMessage());
        }

        double array2[] = new double[numofmp];
        for (int i = 0; i < array2.length; i++)
        {
            array2[i] = array[i];
            // System.out.println(array[i]);
        }

        double result;
        result = 0.0;
        for (int i = 0; i < array2.length; i++)
        {
            result += array2[i];
        }
        average = result / (array2.length);
        System.out.println(numofmp);
        System.out.println("average" + average);

    }// psvm

}// end class

class plotter extends JComponent
{

    final double X_CENTER = 400;
    final double Y_CENTER = 300;
    double x, y;

    public void paint(Graphics g)
    {

        mpplot obj = new mpplot();
        Graphics2D g2 = (Graphics2D) g;
        double radius4 = 150.0f;
        int numpix4 = obj.numofmp;

        double angle4, anginc4;

        anginc4 = (double) (2.0 * Math.PI / numpix4);

        double mparr[] = new double[numpix4];
        for (int i = 0; i < mparr.length; i++)
        {
            mparr[i] = obj.array[i];
        }
        int j = 0;
        for (angle4 = 0.0f; angle4 < 2.0 * Math.PI; angle4 += anginc4)
        {
            x = X_CENTER + (int) (radius4 * Math.cos(angle4));
            y = Y_CENTER + (int) (radius4 * Math.sin(angle4));

            if (j != numpix4)
            {
                if (obj.average - 1.5 < mparr[j]
                        && mparr[j] < obj.average + 1.5)
                {
                    g2.setColor(Color.gray);
                } else
                {
                    if (mparr[j] > obj.average - 1.5)
                    {
                        g2.setColor(Color.magenta);
                    } else
                    {
                        g2.setColor(Color.cyan);
                    }
                }
            }
            g2.setStroke(new BasicStroke(10.0f));
            g2.draw(new Line2D.Double(x, y, x, y));
            j++;

        }

    }// end paint

}// end class

class appletframe extends JApplet
{

    public void init()
    {

        JFrame frame = new JFrame("mp");
        frame.add(new plotter());
        frame.pack();
        frame.setSize(900, 900);
        frame.setVisible(true);
        frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);

        frame.setVisible(true);
        add(frame);

    }

}

[morbiuswilters]

 Your choice of title is redundant.

[tgape]

@subanark said:

Someone asked me why their applet wouldn't work, stated that the error was the applet wasn't public. Aside from this one problem how many other problems can you state with this code? (reformatted for convience)

 

/*

<applet code="appletframe" width=900 height=900>

</applet>

*/

import java.io.;

import java.awt.;

import java.awt.geom.;

import java.awt.geom.Arc2D.;

import javax.swing.*; From what I've read elsewhere on TDWTF, there have been

better alternatives to Swing for many years now.  One.

import java.lang.Double;
class mpplot



{

static double average;
static int numofmp = 0;
<b>static double array[ = new double[800000];</b> <i>Missing close square bracket on

member declaration; member named 'array'.  Three.
public static void main(String args[) throws IOException
{
    String line;

    try
    {
        File file = new File(args[0]);
        final BufferedReader br = new BufferedReader(new FileReader(file));

        double z = 0;

        while ((line = br.readLine()) != null)
        {
            if (line == null)
                break;
            try
            {
                z = Double.parseDouble(line.trim());
                array[numofmp] = z;
                numofmp++;
            } catch (Exception e1)
            {
                System.out.println(e1.getMessage());
            }

        }
    } catch (Exception e2)
    {
        System.out.println(e2.getMessage());
    }

    <b>double array2[ = new double[numofmp];</b> <i>Missing close square bracket on

member declaration; member named 'array2'.  Five.

for (int i = 0; i < array2.length; i++)

{

array2[i] = array[i]; entirely useless duplication of 'array'.  Six.

// System.out.println(array[i]); debugging output commented out, rather

than selected via a debugging flag.  Seven.

}
    double <b>result</b>;
    <b>result = 0.0;
    for (int i = 0; i &lt; array2.length; i++)
    {
        result += array2[i];
    }</b> <i>Computing the total of the numbers in array would have been done more

efficiently in the original loop.  Eight.

average = result / (array2.length);

System.out.println(numofmp);

System.out.println("average" + average); No separation of the string

'average' and the numeric average.  Nine.
}// psvm

}// end class Class with only instantiation and some members, no methods.  Ten.
class plotter extends JComponent

{
<b>final double X_CENTER = 400;
final double Y_CENTER = 300;</b>
double x, y;

public void paint(Graphics g)
{

    mpplot obj = <b>new mpplot();</b> <i>Not being a Java programmer, I'm not

certain about this, but I'd think you'd need to give it an argument.

Graphics2D g2 = (Graphics2D) g;

double radius4 = 150.0f;

int numpix4 = obj.numofmp;
    double <b>angle4</b>, <b>anginc4</b>;

    anginc4 = (double) (2.0 * Math.PI / numpix4);

    <b>double mparr[ = new double[numpix4];</b> <i>Another missing end square

bracket.  It's like you posted code to Community Server or something.  Eleven.

for (int i = 0; i < mparr.length; i++)

{

mparr[i] = obj.array[i]; Senseless duplication of array.  (Admittedly,

array probably should'a been created in this context, or something like that.)

Twelve.

}

int j = 0; J should either be a second loop variable, or Java's TRWTF.

Thirteen.

for (angle4 = 0.0f; angle4 < 2.0 * Math.PI; angle4 += anginc4)

{

x = X_CENTER + (int) (radius4 * Math.cos(angle4));

y = Y_CENTER + (int) (radius4 * Math.sin(angle4));
        if (j != numpix4)
        {
            if (obj.average - 1.5 &lt; mparr[j]
                    && mparr[j] &lt; obj.average + 1.5)
            {
                g2.setColor(Color.gray);
            } else
            {
                if (mparr[j] &gt; obj.average - 1.5)
                {
                    g2.setColor(Color.magenta);
                } else
                {
                    g2.setColor(Color.cyan);
                }
            }
        }
        g2.setStroke(new BasicStroke(10.0f));
        <b>g2.draw(new Line2D.Double(x, y, x, y));</b> <i>Use of a line draw to draw

a point.  Fourteen.

j++;
    }

}// end paint

}// end class
class appletframe extends JApplet

{
public void init()
{

    JFrame frame = new JFrame("mp");
    frame.add(new plotter());
    frame.pack();
    frame.setSize(900, 900);
    frame.setVisible(true);
    frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);

    frame.setVisible(true);
    add(frame);

}

}

If I had Morb's sense of humor, I'd claim the use of Java as one, but that would be lame.  I could've also counted the use of variable++ instead of ++variable, given that there's only one place in this program where variable++ is right, and it's not used there (but is, instead, on the next line).  That would be almost as lame, however.  I counted 23.  I just couldn't bear to keep listing them verbosely, however.  Nor could I bear to complete it, so there's probably a bunch I didn't bold.

[Weng]

@tgape said:

I could've also counted the use of variable++ instead of ++variable

Can someone please give me the explanation as to why the fuck this is true?  I've heard it mentioned over and over again in discussion but have never seen it in print - every book I've read acts like n++ is the One And Only Way To Increment.I think I know what the difference is, but as far as I can tell it's totally irrelevant when you do increments on their own line, which is the way it goes down 99% of the time for readability purposes.

[Heron]

 The missing ] is probably just the forum software breaking, otherwise it wouldn't compile.  (I'm being generous and assuming the original code compiles.)

 For a statement on its own line, there is no behavioral difference between ++var and var++.  The compiled code *may* be different, if the compiler is instructed to ignore every possible optimization, resulting in an extraneous assembly instruction (for compiled languages); I'm not sure if Java even lets you disable that optimization when it compiles the bytecode.

[robbak]

@Weng said:

@tgape said:

I could've also counted the use of variable++ instead of ++variable

Can someone please give me the explanation as to why  this is true?  I've heard it mentioned over and over again in discussion but have never seen it in print - every book I've read acts like n++ is the One And Only Way To Increment.I think I know what the difference is, but as far as I can tell it's totally irrelevant when you do increments on their own line, which is the way it goes down 99% of the time for readability purposes.

 

 

Yes, its irrelevant with built-in types. v++ is (make copy, pass copy as result, increment variable), more complex than ++v (increment variable, pass variable as result). With built-in types, this is going to be optimized away wherever the copy is not required. But it cannot be if you are using a custom type, and the fact that your program will end up working on a copy could be a trap for the unwary. function( badly_written_widget c ){c.setname("name set");} badly_written_widget bww; bww.setname("name unset"); function(bcc++); cout << bcc.name();

So get used to ++v. If you always use it for integers, then you will end up using it for Badly_written_widgets too, and save yourself a debugging session.

 

[bstorer]

@robbak said:

But it cannot be if you are using a custom type, and the fact that your program will end up working on a copy could be a trap for the unwary.

A true statement, but irrelevant in this context.  This particular example is Java, which means you can never use it on anything but a primitive type anyway.

[derula]

@subanark said:

            while ((line = br.readLine()) != null)

{

if (line == null)

break;

I'm not into Java, but how in bloody hell would that if fire? Wouldn't the while condition make sure that line isn't null?

[Guest]

@tgape said:

mpplot obj = new mpplot(); Not being a Java programmer, I'm not certain about this, but I'd think you'd need to give it an argument.

I haven't touched Java in years, but I believe there is a default argumentless constructor unless otherwise specified.

The WTF is instancing a class without any non-static members.

[mrprogguy]

Hrm. I've been out of Java awhile. When did Java add operator overloading?

But it cannot be if you are [i]using a custom type,[/i] and the fact that your program will end up working on a copy could be a trap for the unwary.

Google isn't serving up useful info, and I'm kind of in "lazy Sunday" mode.

[Bulb]

@robbak said:

@Weng said:

@tgape said:

I could've also counted the use of variable++ instead of ++variable

Can someone please give me the explanation as to why  this is true?  I've heard it mentioned over and over again in discussion but have never seen it in print - every book I've read acts like n++ is the One And Only Way To Increment.I think I know what the difference is, but as far as I can tell it's totally irrelevant when you do increments on their own line, which is the way it goes down 99% of the time for readability purposes.

Yes, its irrelevant with built-in types. v++ is (make copy, pass copy as result, increment variable), more complex than ++v (increment variable, pass variable as result). With built-in types, this is going to be optimized away wherever the copy is not required. But it cannot be if you are using a custom type, and the fact that your program will end up working on a copy could be a trap for the unwary. function( badly_written_widget c ){c.setname("name set");} badly_written_widget bww; bww.setname("name unset"); function(bcc++); cout << bcc.name();

So get used to ++v. If you always use it for integers, then you will end up using it for Badly_written_widgets too, and save yourself a debugging session.

That's all fine and right... in C++. This is, however, Java which does not have operator overloads for custom types. Also it has referential semantics for objects, so it couldn't do the copy semantics either.

[pjt33]

@subanark said:

Someone asked me why their applet wouldn't work, stated that the error was the applet wasn't public. Aside from this one problem how many other problems can you state with this code? (reformatted for convience)

mpplot.main is never called. Once you take that into account, mpplot.numofmp is never written to, so remains 0. Therefore numpix4 is always 0. Tracking through the implications of that, plotter.paint is equivalent to

    public void paint(Graphics g)
    {
        Graphics2D g2 = (Graphics2D) g;
        x = 550;
        y = 300;
        g2.setStroke(new BasicStroke(10.0f));
        g2.draw(new Line2D.Double(x, y, x, y));
    }// end paint

mpplot is now entirely redundant and can be deleted. There are still three issues in appletframe.init:

1. The plotter should be added to frame.getContentPane() rather than directly to frame;
2. Since no layout is specified for frame's content pane it's probably not going to size the plotter to anything useful - especially given that it doesn't specify its preferred size in any way;
3. It's pointless calling frame.pack() if you're then going to set its size.

There's also a stupidity in the HTML, because making the applet 900 x 900 when it doesn't paint anything (it opens a separate window instead) is just ugly.

[pjt33]

@tgape said:

I could've also counted the use of variable++ instead of ++variable, given that there's only one place in this program where variable++ is right, and it's not used there (but is, instead, on the next line).

 

This is Java, not C. The compiler produces identical bytecode for x++ and ++x when the increment is a statement by itself.

[Someone_You_Know]

@derula said:

@subanark said:

            while ((line = br.readLine()) != null)
            {
                if (line == null)
                    break;

I'm not into Java, but how in bloody hell would that if fire? Wouldn't the while condition make sure that line isn't null?

 

BufferedReader.readLine() reads the next line from some stream of input, and returns it. If there's nothing left in the stream, it returns null. This is a pretty standard Java idiom.

[Nelle]

@Someone You Know said:

This is a pretty standard Java idiom.

while loop is pretty standard idiom in most programming languages as well ...

[Someone_You_Know]

@Zecc said:

@tgape said:

mpplot obj = new mpplot(); Not being a Java programmer, I'm not certain about this, but I'd think you'd need to give it an argument.

I haven't touched Java in years, but I believe there is a default argumentless constructor unless otherwise specified.

Correct. Java classes automatically have a no-argument constructor that does very little (it calls the no-argument constructor on the superclass, and initializes members to their default values) unless [i]any[/i] other constructor is added manually

[pjt33]

@Someone You Know said:

@derula said:

@subanark said:

            while ((line = br.readLine()) != null)
            {
                if (line == null)
                    break;

I'm not into Java, but how in bloody hell would that if fire? Wouldn't the while condition make sure that line isn't null?

 

BufferedReader.readLine() reads the next line from some stream of input, and returns it. If there's nothing left in the stream, it returns null. This is a pretty standard Java idiom

 You're missing the point. Ignore the readline bit: consider

            while (line != null)
            {
                if (line == null)
                    break;

That's what derula was pointing at while laughing.

[bstorer]

@pjt33 said:

@tgape said:

I could've also counted the use of variable++ instead of ++variable, given that there's only one place in this program where variable++ is right, and it's not used there (but is, instead, on the next line).

 

This is Java, not C. The compiler produces identical bytecode for x++ and ++x when the increment is a statement by itself.

It'll do the same thing in C, and in C++ provided it's a primitive type.

[Someone_You_Know]

@pjt33 said:

 You're missing the point. Ignore the readline bit: consider

            while (line != null)
            {
                if (line == null)
                    break;

That's what derula was pointing at while laughing.

 

Looks like I haven't had enough caffeine this morning...

[Dudehole]

But this is java, so I'm assuming the programmer doesn't have to worry about stupid, confusing shit like copy constructors and operator overloading. So why does this argument apply here?

[Guest]

@Dudehole said:

@robbak said:

Yes, its irrelevant with built-in types. v++ is (make copy, pass copy as result, increment variable), more complex than ++v (increment variable, pass variable as result). With built-in types, this is going to be optimized away wherever the copy is not required. But it cannot be if you are using a custom type, and the fact that your program will end up working on a copy could be a trap for the unwary. function( badly_written_widget c ){c.setname("name set");} badly_written_widget bww; bww.setname("name unset"); function(bcc++); cout << bcc.name();

So get used to ++v. If you always use it for integers, then you will end up using it for Badly_written_widgets too, and save yourself a debugging session.

But this is java, so I'm assuming the programmer doesn't have to worry about stupid, confusing shit like copy constructors and operator overloading. So why does this argument apply here?

FTFY

[SlyEcho]

OK, I'll take a shot at this. I have not done any Java for a quite some time now (once I went C# I never looked back), but I still remember some things.

• No Java naming convention or brace style (this may seem like a trivial point to some, but it is well solidified in the Java community).
• A main entry point in an applet. That takes a file argument. Usually a main method in an applet creates a frame and puts the applet inside it for ease of debugging and development.
• Opening a file but not closing it. It is not hard to add a finally clause to close the file.
• Reading into a fixed array of 800,000 doubles. Why not use ArrayList<double> and let it grow dynamically?
• Copying the original array to a another smaller one. Maybe he thought looping over the large one would be too slow?
• overriding paint() in a JComponent. It is recommended to override paintComponent() instead. This is the Swing way of doing it.
• creating an mpplot object when it only contains static members.
• He expects that the main method has been run, but applets don't run them. Also where would the file name be passed in? How can you read files in an applet running in a browser? (I think it's possible to read resource files from .jar files)
• Not actually using an applet at all, but opening a window¹. Everybody likes pop-ups, right?
• Main point: using Java applets at all for this sort of thing. There is no reason you could not achieve this with Canvas and Javascript. Maybe forcing IE users to switch to another browser is kind of lame but so is forcing them to install the Java Runtime. If you have even better solutions, let's hear them.

¹Can someone confirm that adding a JFrame to a JApplet creates a window?

[pjt33]

@SlyEcho said:

• <snip> How can you read files in an applet running in a browser?

 

You can access the local filesystem if it's a signed applet and the security policy permits it (which it generally does by default).

[morbiuswilters]

@pjt33 said:

@SlyEcho said:

• <snip> How can you read files in an applet running in a browser?

 

You can access the local filesystem if it's a signed applet and the security policy permits it (which it generally does by default).

Sadly, IIRC, Sun dropped support for self-signed certs in Java 6.  This means you have to pay, like, $200 just to get the cert.  I think they changed the default security policy as well.  It's been awhile since I had to mess with applets, so I may be wrong.

[Nelle]

@morbiuswilters said:

Sadly, IIRC, Sun dropped support for self-signed certs in Java 6. 

i cant find anything on google... can anyone confirm this ? it would be a major pita ...

[amischiefr]

@Nelle said:

@morbiuswilters said:

Sadly, IIRC, Sun dropped support for self-signed certs in Java 6. 

i cant find anything on google... can anyone confirm this ? it would be a major pita ...

 

Mmmmmm.... pita

 

[tgape]

@pjt33 said:

@subanark said:

Someone asked me why their applet wouldn't work, stated that the error was the applet wasn't public. Aside from this one problem how many other problems can you state with this code? (reformatted for convience)

mpplot.main is never called

That explains nicely how it is that there isn't a failure because the open attempts to use a variable which isn't passed to the routine.  However, how does it get past the divide by zero?

anginc4 = (double) (2.0 * Math.PI / numpix4);

[pjt33]

 @tgape said:

That explains nicely how it is that there isn't a failure because the open attempts to use a variable which isn't passed to the routine.  However, how does it get past the divide by zero?

anginc4 = (double) (2.0 * Math.PI / numpix4);

It's double division, so the result is Double.POSITIVE_INFINITY.