Konstruktive Kritik zu NetzWrapper

Hi, ich habe mir eine kleine Wrapperklasse geschrieben, die die grundlegende Verbindung macht. Sie funktioniert auch, allerdings ist sie noch lange nicht vollständig. Um Verbesserungen schon am Anfang durchführen zu können, wär es nett, wenn ihr eure Meinung dazu und Verbesserungsvorschläge abgeben könntet.

import java.io.*;
import java.net.*;

public class NetWrapper
{
	protected String m_SIP = "";
	protected int m_iPort = 0;
	protected Socket m_sock = null;	
	protected PrintStream m_ps = null;
	protected BufferedReader m_empfang = null;
	/**
	 * Method Verbinden
	 *
	 *
	 * @return
	 *
	 */
	public boolean Verbinden () throws UnknownHostException, IOException
	{
		try
		{
			m_sock = new Socket(m_SIP, m_iPort);
			m_ps = new PrintStream(m_sock.getOutputStream());
			m_empfang = new BufferedReader(new InputStreamReader(m_sock.getInputStream()));
		}
		catch(IOException e ) {

		}
		return true;
	}

	public String Empfangen(int ab, int zeichen)
	{
		String rueck = "";
		char[] buf = new char[zeichen];
		try
		{
			m_empfang.read(buf, ab, zeichen);
			rueck = rueck.valueOf(buf);
		}
		catch(IOException e){

		}
		return rueck;
	}

	public void Senden(String Nachricht)
	{
		m_ps.println(Nachricht);
	}

	public void NetWrapper(String IP, int Port)
	{
		m_SIP = IP;
		m_iPort = Port;
	}
}

SideWinder

-> Normalerweise programmiert man in der Sprache Englisch und schreibt in Java Methodennamen camelCase und nicht CamelCase

-> Warum sind die Membervariablen protected? Ist das denn wirklich nötig in diesem Fall?

-> PrintStream? Also ich würde da analog zum Lesen einen BufferedWriter benutzen.

MfG SideWinder

Gregor

Nach welcher Art von Kritik suchst Du? Geht es Dir um deinen "Stil" oder eher um das Design der Klasse oder um was?

SideWinder

Ajo BTW: Dein Konstruktor hat den Rückgabetyp void. Glaube nicht, dass das korrekt ist.

MfG SideWinder

Gregor

BTW: Dein Konstruktor hat einen "Rückgabetyp" void. Konstruktoren dürfen aber KEINEN Rückgabetyp haben.

Design der Klasse, aber es ist trotzdem beides willkommen

Gregor

Also, um erstmal die ganze Stil-Geschichte abzuhaken:

[1] Es gibt von Sun Richtlinien bezüglich des Aussehens von Java-Code. Im Großen und Ganzen halten sich die Leute da auch mehr oder weniger dran. Zumindest sind die ein guter Anhaltspunkt bezüglich der Namensgebung, der Formatierung usw. des Codes: http://java.sun.com/docs/codeconv/html/CodeConvTOC.doc.html

[2] Wenn Du etwas weiter bist, also in ein paar Monaten, falls Du in der Zwischenzeit ordentlich programmierst, kannst Du mal in folgendes Buch gucken:
Effective Java | ISBN: 0201310058
Da geht es etwas weitergehend um Stil und um "Design im Kleinen". ...sehr zu empfehlen.

So. Nun ist es wohl so, dass dein Code da oben noch nichtmal kompiliert. Warum postest Du also nicht einfach erstmal die Fehlermeldungen des Compilers und dann gucken wir mal, dass wir zuerst die Fehler rauskriegen.

also bei mir compiliert er problemlos, kann die anwendung auch ausführen ?
die main ist in einer anderen Klasse, die eine Instanz von NetWrapper erzeugt und die Methoden aufruft.
Trotzdem schon mal danke für die Tipps, aus Fehlern lernt man ja(hoffentlich)

Gregor

1. Zeig mal die main-Methode.
2. Welchen Compiler nutzt Du?

import java.io.*;

public class Test
{
	public static void main (String[] args)
	{
		NetWrapper net = new NetWrapper("xxx.xxx.xxx.xxx", 80);
		try
		{
			net.Verbinden();
			net.Senden("GET /blaa HTTP/1.1\r\nhost:www.bla.de\r\n\r\n");
			System.out.println(net.Empfangen(0, 1024));
		}
		catch(IOException e){
			e.printStackTrace();
		}
	}	
}

Compiler, habe ich gestern runtergeladen also 1.5.0_8 oder so ähnlich, glaube ich

Headhunter

Hallo,

hier ein paar Vorschläge:

import java.io.;
import java.net.;

Schöner ist es, wenn du nur die Klassen einbindest, die du auch wirklich brauchst.

- Die Fehlerbehandlung ist mies, alle Fehler werden einfach verschluckt und deine Verbinden Methode wirft auch nie im Leben Fehler.

- "public void NetWrapper(String IP, int Port)" kannst du umwandeln zu "public void NetWrapper (Uri url)", so kannst du dir die Überprüfung von Port & IP auf Richtigkeit sparen.

char[] buf = new char[zeichen];
try
{
m_empfang.read(buf, ab, zeichen);

Du verschwendest Speicher für "buf" wenn "ab" > 0 ist.
Die Variablennamen könnten etwas aussagekräftiger sein imo.

- Die verschiedenen Readers werden nirgends geschlossen. Das ist ein Memoryleak.. am Besten rufst du jeweils die close() Methode in einem finally-konstrukt auf.

- Da man ein Objekt dieser Klasse nur für eine Verbindung benutzen kann: Warum nicht die Funktion von Verbinden() in den Konstruktor packen?

Viele Grüße