🚀 go-pugleaf

Martin Schou <Martin.Schou@Grue-Hornstrup.dk> wrote:
> Pardon the cross-post, and I'm very sorry if I've posted to a wrong group,
> but I'm going nuts here!

I've removed comp.lang.java, as that isn't a valid newsgroup and hasn't 
been for years.

> I'm having two very persistant problems with an applet I'm working on, and I
> have a feeling, that they're related.
> initImages()
>   {
>     MainPanel.picture1 = defineImage( "picture1.gif" );

Yuk - use accessor methods rather making the variable available here.

>         byte[] buffer = new byte[in.available()];
>         in.read(buffer);

Both of these lines are bad. Firstly, you don't know that in.available() 
is going to return the whole file, and secondly, in.read() isn't 
guaranteed to read for a whole buffer. You're ignoring its return value.

See IOUtil at http://www.pobox.com/~skeet/java/skeetutil for some code 
to read a stream completely.

Note that you're also not closing the input stream. That should be done 
within a finally block.

>         System.out.println( adresse + " - " + buffer.length + "\n\r");
>         tempImage = Tk.createImage( buffer );
>       }
>     catch (IOException e)
>       {
>         e.printStackTrace();
>       }
>     catch( NullPointerException E )
>       {
>         E.printStackTrace();
>       }

You should try to pick a naming convention and stick to it. I suggest 
using Sun's - so all of the variables and methods should start with a 
lower-case letter.

Catching NullPointerException is very rarely a good idea

> public void Send( String Message, boolean EndLine )
>   {
>     byte[] message = Message.getBytes();

Never use String.getBytes() - use String.getBytes (String), ie 
specifying an encoding. Use the same encoding as the server will 
understand.

>     catch( Exception E )
>       {
>         E.printStackTrace();
>       }
>   }

Catching bare Exception is almost never a good thing to do - if you want 
to catch everything, use Throwable. If you don't, catch specific 
exceptions.
 
> public void SendEndLine()
>   {
>     try
>       {
>         byte cariageReturn = new String( "\r" ).getBytes()[0];

new String (String) is almost never required, and certainly isn't 
required here. Why not just use DataOut.writeByte ('\r'); in the first 
place though?

> public byte[] ReadAvailable()
>   {
>     byte[] tempByteArray = null;
>     try
>       {
>         int numberOfBytes = DataIn.available();
>         tempByteArray = new byte[ numberOfBytes ];
>         DataIn.read( tempByteArray, 0, numberOfBytes );
>       }
>     catch( IOException E )
>       {
>         Error( E.getMessage() );
>     Sleep( 100 );
>     return tempByteArray;
>   }

Aside from the problems I mentioned earlier, why do you have a call to 
sleep here?
 
> public void ConnectM�ler()
>   {
>     String tempStorage = "";

You never seem to use this variable.

>     tempStorage = "";
>     M�ler.Send( "login", true );//not true login
>     /* Still "fast forwarding" */
> 
>     while( M�ler.ReadAvailable().length > 0 )
>       {
>         M�ler.Sleep( 10 );
>       }

Why are you ignoring what's coming back?
 
>     tempStorage = "";
>     tempStorage = null;

These two lines are unnecessary, even if tempStorage was being used 
somewhere.

-- 
Jon Skeet - <skeet@pobox.com>
http://www.pobox.com/~skeet/
If replying to the group, please do not mail me too