Code Optimierung - Verbesserungen

  • Themenstarter Themenstarter McLovin
  • Beginndatum Beginndatum
M

McLovin

Gast
Hi,

ich habe nun mein Kontaktbuch mit den wichtigsten Funktionen umgesetzt. Allerdings habe ich einige Fragen zu meiner Umsetzung:
1. Das ständige umher kopieren von "phoneBook" dürfte unwahrscheinlich Performancefressend sein, gibt es da eine Möglichkeit das ohne das umherkopieren zu lösen? (Ausser das Telefonbuch statisch zu erzeugen)

2. Wo verbirgt sich der (logische) Fehler in diesem Codefragment?
Code:
System.out.println("Insert the new Tel.(otherwise leave it empty): ");
if ( (tel = sc.next()) != "")
   phoneBook.get(index-1).setTelefonNumber(tel);

3. Arbeite ich richtig mit der ArrayList?

5. Gibt es ansonsten noch Ratschläge wie ich etwas eleganter hätte lösen können oder Auffälligkeiten?

Hier meine Quelltexte:

Contact.java
Code:
public class Contact
{
	private String name;
	private String telefonNumber;
	
	Contact(String name, String telefonNumber)
	{
		this.name = name;
		this.telefonNumber = telefonNumber;
	} 
	
	/**
	 * Sets the (new) name of the contact.
	 * @param:	new name of the contact
	 */
	public void setName(String newName)
	{
		this.name = newName;
	}
	
	/**
	 * Sets the (new) telefonnumber of the contact.
	 * @param:	new telefonnumber of the contact
	 */
	public void setTelefonNumber (String newTel)
	{
		this.telefonNumber = newTel;
	}
	
	/**
	 * Returns the name of the contact.
	 * @return: name
	 */
	public String getName () 
	{
		return this.name;
	
	}
	/**
	 *	Returns the telefonnumber of the contact. 
	 * 	@return: telefonNumber
	 */
	public String getTelefonNumber() 
	{
		return this.telefonNumber;
	}
	
	
	
}

PhoneBook.java
Code:
import java.util.*;

public class PhoneBook
{	
	static Scanner sc = new Scanner(System.in); // Scanner-Object to interact with user
	static void showMenu()
	{
		System.out.println("new contact    <1>");
		System.out.println("show contacts  <2>");
		System.out.println("edit contact   <3>");
		System.out.println("delete contact <4>");
		System.out.println("exit           <0>");
		System.out.println("------------------");
	}

	static Contact newContact()
	{		
			String name;
			String tel;
			
			System.out.print("Insert the name of your contact: ");
			name = sc.next();
			System.out.print("Insert the telefonnumber of your contact: ");
			tel = sc.next();
			
			Contact con = new Contact(name, tel);
			System.out.println("Successfully created a new contact!\n");
			
			return con;
	}
	
	static void printPhoneBook(ArrayList<Contact> phoneBook, int numberContact)
	{
		for (int i = 0; i<numberContact;i++)
		{
			System.out.print( (i+1) + ": " + phoneBook.get(i).getName()+ " ");
			System.out.println("Tel.: " + phoneBook.get(i).getTelefonNumber());
		}
		
	}
	
	static ArrayList<Contact> editContact(ArrayList<Contact> phoneBook)
	{
		int index;
		String name, tel;
		System.out.println("Which contact do you want to edit? :");
		index = sc.nextInt();
		
		System.out.println("Insert the new name(otherwise leave it empty): ");
		if ( (name = sc.next()) != "")
			phoneBook.get(index-1).setName(name);
		System.out.println("Insert the new Tel.(otherwise leave it empty): ");
		if ( (tel = sc.next()) != "")
			phoneBook.get(index-1).setTelefonNumber(tel);
			
		return phoneBook;		
	}
	
	static ArrayList<Contact> deleteContact(ArrayList<Contact> phoneBook)
	{
		int index;
		System.out.print("Which contact do you want to delete? :");
		index = sc.nextInt();
		System.out.print("Do you really want to delete " + phoneBook.get(index-1).getName() + "(j/n)");
		if (sc.next() == "j")
		{
			phoneBook.remove(index-1);
		}
		
		return phoneBook;
			
	}
	 
	public static void main(String[] args)
	{
		
		ArrayList<Contact> phoneBook = new ArrayList<Contact> (5);
		int input = 1337;
		int numberContacts=0;
		
		System.out.println("Welcome to your phonebook!");
		do
		{
			showMenu();
			input = sc.nextInt();
			
			switch(input)
			{
			case 1:
					System.out.println("");
					phoneBook.add(newContact());
					numberContacts++;
					break;
			case 2: 
					System.out.println("");
					printPhoneBook(phoneBook, numberContacts);
					break;
			case 3:
					editContact(phoneBook);
					break;
			case 4: 
					System.out.println("");
					phoneBook = deleteContact(phoneBook);
					numberContacts--;
					break;
			}
			
		} while(input !=0);
	}
}

PS: Ja, bei PhoneBook war ich zu faul zu kommentieren 🙁

Gruß,
McLovin
 
1.
Also da du PhoneBook eigentlich gar nicht als Objekt hats, sondern nur statische Methoden aufrufst, weiß ich nicht genau, was du da mit "umherkopieren" meinst.

2.
if ( (tel = sc.next()) != "")
Da prüfst du auf Referenzgleichheit. Die wird wohl nicht gegeben sein, daher kriegst du auch false zurück.
 
Zuletzt bearbeitet:
1.
Also da du PhoneBook eigentlich gar nicht als Objekt hats, sondern nur statische Methoden aufrufst, weiß ich nicht genau, was du da mit "umherkopieren" meinst.

Ah, mein Fehler. Ich meinte die ArrayList "phoneBook" -> die Benennung sollte ich vielleicht noch ändern.

2.
if ( (tel = sc.next()) != "")
Da prüfst du auf Referenzgleichheit. Die wird wohl nicht gegeben sein, daher kriegst du auch false zurück.

Danke. Habe direkt mal danach gegooglet - Strings werden mit .equals() vergliechen. Die Umsetzung des "Gib neuen Namen ein, ansonsten leer lassen" kriege ich dennoch nicht hin. Habe dann versucht es mit ".hasNext()" zu lösen - was auch bedingt funktioniert hat, die alte Telefonnummer wird also nicht überschrieben. Allerdings führt das "leer" lassen der Eingabe nicht, wie erwartet, dazu, dass das Bearbeiten des Kontaktes beendet und das Menü angezeigt wird - die Nächste "richtige" Eingabe(1-4) des Nutzers wird vom Programm jedoch als "Menüanweisung" erkannt und das Programm handelt korrekt darauf:

Code:
		if ( !(sc.hasNext()) )
		{
			tel = sc.next();
			phoneBook.get(index-1).setTelefonNumber(tel);
		}

Gruß,
McLovin
 
Hi,

ich habe meinen Quellcode nun nocheinmal überarbeitet und die Arraylist als statisch erzeugt und mit einem anderen Namen(="contacts") versehen um den Code etwas übersichtlicher zu gestalten und das ständige umherkopieren der ArrayList zu vermeiden. Nun habe ich aber immernoch mit zwei Methoden starke Probleme:

Code:
static void editContact()
	{
		int index;
		
		String name, tel;
		System.out.println("Which contact do you want to edit? :");
		index = sc.nextInt()-1;
		
		System.out.println("Insert the new name(otherwise leave it empty): ");
		if ( !(sc.hasNext()) )
		{
			name = sc.next();
			contacts.get(index).setName(name);
		}
		
		System.out.println("Insert the new Tel.(otherwise leave it empty): ");
		if ( !(sc.hasNext()) )
		{
			tel = sc.next();
			contacts.get(index).setTelefonNumber(tel);
		}		
	}
Die Funktion funkioniert einwandfrei - insofern der Kontakt komplett umgeändert werden soll die Funktion "(otherwise leave it empty)" wird komplett übergangen.


Code:
static void deleteContact()
	{
		int index;
		
		System.out.print("Which contact do you want to delete? :");
		index = (sc.nextInt()-1);
		System.out.print("Do you really want to delete " + contacts.get(index).getName() + "? (j/n)");
		if (sc.next() == "j")
		{
			for(int i = 0; i<contacts.size();i++)
			{
				contacts.set( index+i, (contacts.get( (index+i+1) ) ) );
			}
			contacts.remove( (contacts.size()-1) );
			System.out.println("Successfully removed the contact!\n");
		}
			
	}
In der ArrayList contacts wird entgegen meinen Erwartungen der gewünschte Eintrag nicht überschrieben und der letzte Eintrag nicht gelöscht (Anders gesagt: Es geschiet nichts). Mit dem übergebenen Index dürfte das eigentlich nicht zusammenhängen, da das Programm den Namen des Kontaktes korrekt erkennt.
 
Das Problem mit der Methode editContact() habe ich nun anders gelöst. Um den Namen bzw. die Tel. beizubehalten wird nun von Program die Eingabe "/e" erwartet - mich würde dennoch interessieren, wie ich die nächste Eingabe ignoriere, wenn die leer ist.

Bei der Methode removeContact() hatte ich zwei triviale Fehler: Strings nicht mit .equals() verglichen und einen Indexfehler.
 
McLovin hat gesagt.:
das ständige umherkopieren der ArrayList zu vermeiden
Java ist nicht wie C++, wo man ständig aufpassen muss, was genau man jetzt hin und her schiebt; wenn du nur ein Objekt konstruierst (=
Code:
new
), gibt es auch wirklich nur ein Objekt!
Alle Parameter sind nur Kopien der Referenzen, also total vernachlässigbar in Sachen performance; vor allem als Anfänger (1. Schritt: Bring es zum laufen; 2. Schritt (nur für Profis): Mach es schneller).

Dann noch als Verbesserungsvorschläge:

1. All dein static raus, das zerstört das OO.
2. Ich persönlich verwende bei IO immer nur die
Code:
readLine
-Methode; so muss ich zwar die Eingabe parsen, aber habe keine Probleme mit den versteckten Zeichen (also zum Beispiel der line separator, welcher ja bei jedem Bestätigen der Eingabe mit kommt)
 
Java ist nicht wie C++, wo man ständig aufpassen muss, was genau man jetzt hin und her schiebt; wenn du nur ein Objekt konstruierst (=
Code:
new
), gibt es auch wirklich nur ein Objekt!

Danke - genau das war mein Problem! Da ist Java im Vergleich mit C++ deutlich komfortabler.

1. All dein static raus, das zerstört das OO.

Danke auch hierfür - hatte das total aus den Augen verloren und war voll und ganz darauf fixiert das Telefonbuch zum laufen zubekommen. Habe hierzu auch direkt einen guten "Merksatz" auf Wipedia gefunden:
Code:
Entscheidend ist, dass bei dem jeweiligen Objektbegriff eine sinnvolle und allgemein übliche Zuordnung möglich ist.

2. Ich persönlich verwende bei IO immer nur die
Code:
readLine
-Methode; [...]

Die Methode werde ich mir noch genauer anschauen - mein Lehrbuch hatte (fast) ausschließlich mit
Code:
next
gearbeitet. Die Tage kommt aber mein neues Java-Handbuch, da kann ich dann direkt mal nachschlagen.
 

Zurück
Oben