2 replies to this topic

[Skylark320]

Hey guys, trying to build a basic communication program for teachers at school to report problems in class etc, as my A2 project. However, hit a problem that i can't see is a coding error and wondered if you guys could help...

basically, in my 'TeacherServer class i run the typical Frame.setVisible(true) method as is implemented many times in my program'

public static void Listen() throws IOException, SQLException

{

    //Creates a server socket to listen to the DutyTeacher

ServerSocket mySS = new ServerSocket(5001);

Socket ss_accept = mySS.accept();

BufferedReader ss_BF = new BufferedReader(new InputStreamReader(ss_accept.getInputStream()));

String Attending = ss_BF.readLine();


//Checks content of Duty Messafe

if(Attending.contentEquals("Attending"))

{

    //If they leave the box blank, they are attending, open JoptionPane

JOptionPane.showMessageDialog(null, "The Duty Teacher is attending");

mySS.close();

ss_accept.close();

ss_BF.close();

System.exit(1);

}else

    //If the box is full a conversation is started, opens conversation box   

{if(Attending.contentEquals("Conversation"))

{

new ConversationTeacher().setVisible(true);   <--------------------------------------------------

mySS.close();

ss_accept.close();

ss_BF.close();

Listen();

 }else{

     //If neither of the others, it is a new convo message, conversation updated

ConversationTeacher.UpdateConversation();

mySS.close();

ss_accept.close();

ss_BF.close();

Listen();

 }}

}

however i simply get a frame with a blank interior. it runs fine if i run the frame on it's own, but won't open from this teacher server class.

public class ConversationTeacher extends javax.swing.JFrame {


    /** Creates new form Conversation */

    public ConversationTeacher() throws SQLException {

        initComponents();

        //Sets the conversation box with the first message from the duty teacher

        ConversationConversation.setText("Hey");

    }

ConversationConversation is a text field

Thanks in advance

Ben

Edit: ConversationConversation is being set to hey but would usually search a sql server database for the conversation details.
Any comments on my general code structure etc is also much appreciated :c-^_^:

[lethalwire]

I'm not at my desk to test this, but what if you add

super()

to your constructor. (for the class that implements Jframe)

Have you considered extending jdialog instead of jframe?
Usually, if I'm opening new windows, I'll extend jdialog.

[wim DC]

Could you also post the initComponents() section?
If it does what the method says: just X y = new X(); a bunch of times then maybe you forgot adding y to the JFrame: add(y)
Or super.add(y) If that makes more sence to you. It does the same though.

As the previous poster said you should also put super(); as the first line in your constructor.
However I think the compiler may do that for you automatically. Better do it yourself tho.

As for comments on general structure:

public static void Listen() throws IOException, SQLException

{

    //Creates a server socket to listen to the DutyTeacher

ServerSocket mySS = new ServerSocket(5001);

Socket ss_accept = mySS.accept();

BufferedReader ss_BF = new BufferedReader(new InputStreamReader(ss_accept.getInputStream()));

String Attending = ss_BF.readLine();


//Checks content of Duty Messafe

if(Attending.contentEquals("Attending"))

{

    //If they leave the box blank, they are attending, open JoptionPane

JOptionPane.showMessageDialog(null, "The Duty Teacher is attending");

[B][COLOR="red"]mySS.close();

ss_accept.close();

ss_BF.close();[/COLOR][/B]

System.exit(1);

}else

    //If the box is full a conversation is started, opens conversation box   

{if(Attending.contentEquals("Conversation"))

{

new ConversationTeacher().setVisible(true);   <--------------------------------------------------

[B][COLOR="red"]mySS.close();

ss_accept.close();

ss_BF.close();[/COLOR][/B]

Listen();

 }else{

     //If neither of the others, it is a new convo message, conversation updated

ConversationTeacher.UpdateConversation();

[B][COLOR="red"]mySS.close();

ss_accept.close();

ss_BF.close();[/COLOR][/B]

Listen();

 }}

}

DRY: don't repeat yourself.
Put that in a method and call the method. I'm not sure if the sockets / readers are still being used or not..
maybe you can close them oustside the if statement allready, the String has been read allready anyway.

It also "feels" kinda wrong to first close the server and then the client. I would close it in the reverse way you created it.
So first reader, then (client)Socket then ServerSocket.

Then there is

if( Attending.contentEquals("Attending") )

(variables should start with a small letter)
But why use contentEquals() instead of equals()? contentEquals takes a StringBuffer as parameter
so I think java will have to cast your string to a stringbuffer which will take up resources. (Noone cares, but still :c-smile:)

The attending String is read with : String Attending = ss_BF.readLine();
Javadoc says the following about the readLine():

Quote

A String containing the contents of the line, not including any line-termination characters, or null if the end of the stream has been reached

The "or null"-part may be somewhat annoying because then your program will crash. To avoid this you can change the equals around as followed:

if( "Attending".equals(attending) )

If attending (the object), is null now it will just be false in the if-statement instead of crashing.