Wątek przeniesiony 2015-05-03 12:17 z Java przez bogdans.

Ocena kodu prostego etytora tekstowego

0

napisałem prosty edytor w java nie zrobiłem jescze funkcji zapisz ale chciałbym żebyście zobaczyli i ocenili a w razie czego poprawili błedy

 
import java.awt.BorderLayout;
import java.awt.Component;
import java.awt.Container;
import java.awt.Dimension;
import java.awt.GridLayout;
import java.awt.event.ActionEvent;      
import java.awt.event.ActionListener;
import java.io.BufferedReader;                       
import java.io.BufferedWriter;
import java.io.File;
import java.io.FileReader;
import java.io.FileWriter;
import java.io.IOException;

import javax.swing.JButton;
import javax.swing.JFileChooser;
import javax.swing.JFrame;
import javax.swing.JTextArea;
public class Edytor extends JFrame implements ActionListener {
private JButton wczytaj=new JButton("wczytaj");
private JButton zapisz=new JButton("zapisz");
private JTextArea edytor=new JTextArea();
public Edytor()
{
Container c = this.getContentPane();
Container c2 = new Container();
c2.setLayout(new GridLayout());
c.setLayout(new BorderLayout());
//c.add(przycisk,BorderLayout.SOUTH);
c2.add(wczytaj,BorderLayout.EAST);
c2.add(zapisz,BorderLayout.WEST);
c.add(c2,BorderLayout.SOUTH);
c.add(edytor,BorderLayout.CENTER);
wczytaj.addActionListener(this);
this.setSize(new Dimension(600,600));
this.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
this.setVisible(true);
}
public static void main(String[] args) {
	new Edytor();
}
@Override
public void actionPerformed(ActionEvent a) {
if(a.getSource()==wczytaj)
{
	
		JFileChooser fc =new JFileChooser();
		fc.showOpenDialog(null);
		String tekst = "";
		File plik=fc.getSelectedFile();
		try {
			BufferedReader br=new BufferedReader(new FileReader(plik));
			String linia;
			do
			{
				linia=br.readLine();
				if(linia!=null)
					tekst+=linia+"\n";
			}while(linia!=null);
			
			br.close();
		} catch (IOException e) {
			// TODO Auto-generated catch block
			e.printStackTrace();
		}
		System.out.print(tekst);
	}
	edytor.append("wczytaj");
	System.out.println("zapisz");

if(a.getSource()==zapisz);
{
	
System.out.println(edytor.getText());
}
}
} 
0

Kilka uwag:

  1. Formatuj porządnie kod.
  2. Próbujesz czytać plik, mimo że użytkownik zrezygnował (kliknął Cancel w okienku dialogowym).
  3. Nigdy nie używaj operatora konkatenacji do sklejania wielu Stringów - jest on bardzo wolny.
StringBuilder tekst = new StringBuilder("");
...
String linia;
do
{
     linia=br.readLine();
     if(linia!=null)
         tekst.append(linia+"\n");
}while(linia!=null);
...
edytor.append(tekst);
0
  1. Niekonsekwentne formatowanie.
  2. Raz są wcięcia, raz ich nie ma.
  3. Raz są odstępy po operatorach, raz ich nie ma
  4. Prawie każde IDE ma automatyczne formatowanie kodu - np. w IntelliJ masz skrót: Ctrl+Alt+L
  5. "Zakomentowany" kod - do usunięcia
  6. Nic nie mówiące nazwy zmiennych typu: a, c, c2
  7. Brak nowych linii w niektórych miejscach redukuje czytelność kodu - warto dodać nową linię przed deklaracją metody i po deklaracji metody oraz rozdzielić kluczowe fragmenty kodu
  8. Miks polskiego języka z angielskim. Używaj tylko angielskich nazw w kodzie. Taki jest standard. Czasami pracuje się w międzynarodowych zespołach i każdy powinien rozumieć kod.
  9. Stringi typu "wczytaj", "zapisz", "", "\n" oraz niezmienne wartości typu 600 - możesz wstawić do zmiennych statycznych
  10. tekst+=linia+"\n"; - dodawanie linii w ten sposób jest niewydajne - przy dużym pliku będzie to trwało bardzo długo. Użyj StringBuildera, jak polecił @bogdans.
  11. // TODO Auto-generated catch block - niepotrzebny komentarz - do usunięcia; warto wstawić tam jakąś obsługę błędów - np. wyświetlenie okienka dialogowego z informacją o błędzie, zalogowanie zdarzenia, etc.
  12. a.getSource()==wczytaj - sprawdzasz tutaj, czy referencja do dwóch obiektów jest taka sama zamiast porównywać zawartość obiektów. Czy na pewno to chcesz robić? Poczytaj o różnicach między == i equals(). Dodatkowo możesz ubrać tę instrukcję w jakąś deskryptywną nazwę - np. boolean onReadButtonClicked = a.getSource()==wczytaj. Wtedy od razu wiemy, o co chodzi.
  13. Nie wiadomo, co metoda actionPerformed() tak naprawdę robi bez analizowania kodu. Powinieneś ubrać ten kod w jakąś metodę typu readFile() i wstawić ją do actionPerformed().
  14. Powinieneś też rozdzielić czytanie i wyświetlanie na 2 osobne metody. Może w przyszłości będziesz chciał dodać np. funkcję drukowania, to wtedy unikniesz powtórzeń i użyjesz metody do czytania pliku.

1 użytkowników online, w tym zalogowanych: 0, gości: 1