Was haltet ihr von meinem Quellcode - JConsole?

[Dr.House]

Schönen guten Abend,

und zwar hab ich eine Console geschrieben und wollte mal wissen was ihr davon haltet - gibt es Lags oder irgendwelche Unschönen Dinge? Verbesserungen?

/*
 * Created on 07.06.2007
 *
 * TODO To change the template for this generated file go to
 * Window - Preferences - Java - Code Style - Code Templates
 */

/**
 * @author &KFmobile
 *
 * TODO To change the template for this generated type comment go to
 * Window - Preferences - Java - Code Style - Code Templates
 */

import javax.swing.*;
import java.awt.Font;
import java.awt.Color;
import java.awt.GridLayout;

import java.awt.*;
import javax.swing.text.*;


public class JConsoleFrame extends JFrame
{
	private int			msg_cnt			= 0;
	private boolean 	bIniTextpane	= false;
	private JTextPane 	textArea 		= new JTextPane();
	
	public JConsoleFrame()
	{
		setTitle( "Console" );
		
		//textArea = new JTextPane();
		//textArea.disable();
		textArea.setEditable(false);
		//textPane.insertIcon(new Icon("test.gif"));
		getContentPane().add( textArea );
		bIniTextpane = true;
		
		textArea.setForeground(Color.blue);
		
		
		// define scroll bar
		JScrollPane scrollingArea = new JScrollPane( textArea );
	
		// define panel for scrollbar 
        JPanel content = new JPanel();
        content.setLayout(new BorderLayout());
        content.add(scrollingArea, BorderLayout.CENTER);
        setContentPane(content);        
        
        textArea.setFont( new Font("Courier New", Font.PLAIN, 14 ) );
        
        // add starting text
        addText ( "Starting console" );
		pack();
		
		// resize frame
		setSize( 600, 500 );
	}

	
	public void addLine( String line )
	{
		if( bIniTextpane )
		{
			addText( "\n> "+line );
			msg_cnt++;
		}
		else
		{
		}
	}
	
	
	public void addLine( String line, Color color )
	{
		if( bIniTextpane )
		{
			addText( "\n> "+line, color );
			msg_cnt++;
		}
		else
		{
		}
	}
	
	
	public void addError( String line )
	{
		//this.show();
		
		if( bIniTextpane )
		{
			addText( "\n> ERROR: "+line, new Color(255,0,0) );
			msg_cnt++;
		}
		else
		{
		}
	}	
	
	
	public void addText ( String text ){
		addTextEx( text, Color.BLACK, false );
	}
	
	
	public void addText ( String text, Color color )
	{
		try{
			addTextEx( text, color, false );
		}catch( Exception e){System.err.println("*ERROR: [Console::addText()]"+e.getMessage()); }
	}
	
	
	public void addTextEx ( String text, Color color, boolean underlined )
	{
		msg_cnt++;
		if( msg_cnt > 2000 ){
			msg_cnt = 0;
			clear();
		}
		
		try{
			// fetch styledocument from textpane
			StyledDocument doc = (StyledDocument)textArea.getDocument();
			
			// add style
			Style style = doc.addStyle( null, null);
			
			// set text as underlined
			if( underlined ) StyleConstants.setUnderline(style, true);
			
			// set text color
			StyleConstants.setForeground(style, color );
			
			// add string 
			doc.insertString( doc.getLength(), text, style);
			
			// scroll down
			textArea.setCaretPosition( textArea.getDocument().getLength() );
		
		}catch( Exception e){System.err.println("*ERROR: [Console::addTextEx()]"+e.getMessage());}
		
	}	
	
	public void clear(){
		try{
			// clear
			StyledDocument document = (StyledDocument)textArea.getDocument();
			document.remove( 0, document.getLength() );
		}catch( Exception e ){System.err.println("*ERROR [Console::Clear()]: "+e.getMessage());}
		
		addTextEx( "Console cleared", new Color(0,0,255), false );
	}
	
};

 

[tfa]

Hier ein paar Sachen, die mir aufgefallen sind:

• 
  * Der Quelltext ist nicht dokumentiert und nur schlecht kommentiert.
  * Wozu die leeren else-Blöcke? Die sind überflüssig.
  * Du solltest eine Konstante für das Color-Objekt, das in addError verwendet wird anlegen. Dann wird nicht jedesmal ein neues Objekt erzeugt.
  * Nach 2000 Einträgen wird die Konsole gelöscht? Das ist ja fies! Was, wenn ich genau den 1999. sehen will? Du solltest nur die ältesten Einträge löschen, wenn es zu viel wird.
  * Java-Coding-Conventions sind verletzt (Unterstrich in msg_cnt). Das b vor bIniTextPane soll wohl Ungarische Notation sein. Sowas ist im Allgemeinen auch nicht gern gesehen. Wozu überhaupt diese Variable?
  * Das Exception-Handling ist etwas unglücklich. catch(Exception) sollte man vermeiden, statt dessen nur konrete Exeption auffangen und verarbeiten. Die Exception-Behandlung in der mittleren addText-Methode ist überflüssig.

 

[Guest]

msg_cnt wird zu oft inkrementiert.
Ansonsten, was ist, wenn man nach dem 1999sten Eintrag mehr als eine Fehlermeldung ausgibt? Nur die letzte
kriegt man zu sehen, der Rest wird einfach verworfen. Verwende lieger eine vernünftigen Logger und einen
LogViewer, statt das Rad neu zu erfinden.