2011-03-12 3 views
1

J'aimerais que quelqu'un regarde le code suivant et dis-moi s'il y a quelque chose qui cloche/bizarre. Je suis à la recherche de conseils généraux en codage. Si vous pouvez simplement me diriger dans la bonne direction, ce serait formidable.Guide de programmation Java

Je suis très nouveau à Java. J'ai écrit et réécrit les dizaines de fois suivantes et ça marche bien. Je veux juste que quelqu'un jette un oeil pour voir si quelque chose apparaît comme manifestement faux.

import net.contentobjects.jnotify.JNotify; 
import net.contentobjects.jnotify.JNotifyListener; 
import java.io.*; 
import java.text.SimpleDateFormat; 
import java.util.Calendar; 
import java.awt.Dimension; 
import java.awt.Toolkit; 
import java.awt.*; 
import javax.swing.JTextArea; 
import javax.swing.JFrame; 
import javax.swing.JScrollPane; 
import java.awt.event.ActionEvent; 
import java.awt.event.ActionListener; 
import javax.swing.*; 
import javax.swing.JLabel; 

public class FileWatcher implements ActionListener { 
    private JTextArea textArea = new JTextArea(); 
    private JScrollPane pane = new JScrollPane(textArea); 
    private Dimension screenSize = Toolkit.getDefaultToolkit().getScreenSize(); 
    private JFrame frame = new JFrame(); 
    private JButton startButton = new JButton("Clear"); 
    private JPanel panel = new JPanel(); 

    public FileWatcher(String path) { 
     Font font = new Font("Arial Unicode MS", Font.PLAIN, 13); 
     this.textArea.setEditable(false); 
     this.frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE); 
     this.pane.setHorizontalScrollBarPolicy(JScrollPane.HORIZONTAL_SCROLLBAR_NEVER); 
     this.frame.add(pane); 
     this.frame.setSize(500, 200); 
     this.frame.setLocation(screenSize.width - 500, screenSize.height - 250); 
     this.frame.setTitle("Monitoring: " + path + "..."); 

     this.textArea.setFont(font); 
     this.frame.add(panel, BorderLayout.SOUTH); 
     startButton.setPreferredSize(new Dimension(450, 20)); 
     this.panel.add(startButton); 
     this.startButton.addActionListener(this); 
     this.frame.setVisible(true);   
    } 

    private void sample(String path) throws Exception { 
     int mask = JNotify.FILE_CREATED | JNotify.FILE_DELETED 
       | JNotify.FILE_MODIFIED | JNotify.FILE_RENAMED; 
     boolean watchSubtree = true; 
     JNotify.addWatch(path, mask, watchSubtree, new Listener()); 
    } 

    public void actionPerformed(ActionEvent e) { 
     this.textArea.setText("");  
    } 

    public void TouchFile(String fName) { 
     if ((fName.indexOf("Thumbs") == -1) && (fName.indexOf(".rar") == -1) 
       && (fName.indexOf(".part") == -1)) { 
      String TodayDate = ""; 
      String fModifyDate = ""; 
      String line; 
      String line2 = "error"; 
      Calendar calendar = Calendar.getInstance(); 
      SimpleDateFormat dateFormat = new SimpleDateFormat("dd/MM/yyyy"); 
      File filename = new File(fName); 
      try { 
      } catch (Exception e) { 
       e.printStackTrace(); 
      } 
      TodayDate = dateFormat.format(calendar.getTime()); 
      fModifyDate = dateFormat.format(filename.lastModified()); 
      try { 
       Thread.currentThread().sleep(1000); 

       while ((line2.indexOf("error") != -1) 
         && (!fModifyDate.equals(TodayDate))) { 
        line2 = " "; 
        line = " "; 
//     Process p = 
//     Runtime.getRuntime().exec("C:\\Users\\BLUE\\Documents\\MYDROP~1\\Utils\\WatchFolder\\touch.exe " 
//     + "\"" + fName + "\""); 
        Process p = Runtime.getRuntime().exec("touch.exe " + "\"" + fName + "\""); 
        BufferedReader input = new BufferedReader(
          new InputStreamReader(p.getInputStream())); 
        while ((line = input.readLine()) != null) { 
         line2 = line2 + line; 
         showInfo(line); 
        } 
        input.close(); 
        p.waitFor(); 
       } 
      } catch (Exception err) { 
       err.printStackTrace(); 
      } 
     } 
    } 

    String chopData(String data) { 
     File file = new File(data); 
     String fileName= file.getName(); 
     if (data.indexOf("Touched") != -1) { 
      int tIndex = data.indexOf("Touched"); 
      data = data.substring(0,tIndex+8) + "..." + fileName; 
     } 

     if ((data.indexOf("Touched") == -1) && (data.length() > 70)) { 
      data = data.substring(0, data.indexOf(" ")) + " ... " 
        + data.substring(data.length() - 55); 
     } 
     return data; 
    } 

    public void showInfo(String data) { 
     data = chopData(data); 
     this.textArea.append(data + "\n"); 
     this.frame.getContentPane().validate(); 
     this.textArea.setCaretPosition(this.textArea.getText().length()); 
    } 

    class Listener implements JNotifyListener { 
     public void fileRenamed(int wd, String rootPath, String oldName, String newName) { 
      showInfo("renamed ..." + oldName.substring(oldName.length() - 25) 
        + " -> ..." + newName.substring(newName.length() - 25)); 
     } 

     public void fileModified(int wd, String rootPath, String name) { 
      TouchFile(rootPath + "\\" + name); 
     } 

     public void fileDeleted(int wd, String rootPath, String name) { 
      showInfo("deleted " + name); 
     } 

     public void fileCreated(int wd, String rootPath, String name) { 
      TouchFile(rootPath + "\\" + name); 
      showInfo("created " + name); 
     } 

     void print(String msg) { 
      showInfo(msg); 
     } 
    } 

    public static void main(String[] args) { 

     try { 
      if (args.length > 0) { 
       FileWatcher[] fwArray = new FileWatcher[args.length]; 
       for (int i = 0; i < args.length; i++) { 
        fwArray[i] = new FileWatcher(args[i]); 
        fwArray[i].sample(args[i]); 
       } 

      } else { 
       FileWatcher watcher = new FileWatcher("H:\\download"); 
       watcher.sample("H:\\download"); 

      } 

     } catch (Exception e) { 
      e.printStackTrace(); 
     } 
    } 


} 

Répondre

3

blocs comme

try { 
     } catch (Exception e) { 
      e.printStackTrace(); 
     } 

chemins d'accès directement

FileWatcher("H:\\download"); 

nombres magiques

if ((data.indexOf("Touched") == -1) && (data.length() > 70)) { 
     data = data.substring(0, data.indexOf(" ")) + " ... " 
       + data.substring(data.length() - 55); 

Non optimisé importations

import javax.swing.*; 
import javax.swing.JLabel; 
+0

Merci beaucoup! Ceux-ci ont pris quelques-uns en levant les yeux mais je l'ai maintenant. Merci de votre aide. – zincc

2
public void TouchFile(String fName) 

Le nom de la méthode est incorrect. Vous ne devez PAS mettre en majuscule le premier caractère.

TodayDate = dateFormat.format(calendar.getTime()); 

Varialbe le nom est faux. Vous ne devez PAS mettre en majuscule le premier caractère.

La cohérence est importante lors de l'écriture de code. Suivez les conventions standard.

+0

Merci! J'étais malpropre. – zincc

1

Thread.currentThread().sleep(1000);

devrait être

Thread.sleep(1000);

!fName.contains("Thumbs")

au lieu de

fName.indexOf("Thumbs") == -1

+0

Merci! Je pensais qu'il devait y avoir une façon plus simple de faire cela. – zincc

1

Créer plus cla sses - 1ère classe - présentation, 2ème opérations d'E/S. Déplacer classe interne vers un fichier séparé, créer un paquet

méthode Make TouchFile plus simple - diviser sur des parties plus petites, le nom du changement touchFile

Supprimer le code commenté, nombres magiques, créer des chaînes finales avec des chemins ("H: \ download «) et le texte

Je pense que cette condition n'est pas ce que vous attendez:

if ((fName.indexOf("Thumbs") == -1) && (fName.indexOf(".rar") == -1) 
    && (fName.indexOf(".part") == -1)) 

mieux:

fName.toLowerCase().endsWith(".rar") ... 
+0

Hey! Merci à tous. C'est beaucoup plus de commentaires et de conseils que ce que je pensais obtenir. – zincc

2

HI Zincc, Bienvenue dans StackExchange et bienvenue dans Java.

échange Stack a un site spécifique en version bêta en ce moment pour des questions comme celle-ci, vous voudrez peut-être jeter un oeil: https://codereview.stackexchange.com/

Aussi je vous suggère d'utiliser la déclaration d'importation en utilisant les noms de classe qualifiés:

import java.awt.Toolkit; 

et non comme:

import java.awt.*; 

comme il fera le processus de compilation prendre un peu plus de temps, pas vraiment une grosse affaire, mais vous peut lire cet article: Java Performance Tuning

Cordialement! Jhurtado

+0

wow, merci pour codereviw url - Je ne savais pas sur ce site avant - semble incroyable – smas

+0

Oui! il est en statut bêta, donc si vous l'aimez, rejoignez et participez! :) – jhurtado

+0

Merci pour le lien et les suggestions! :) – zincc

1

Portée variable. Les variables doivent être déclarées correctement lorsqu'elles ne sont pas toutes utilisées en haut dans un grand style C++. Dans chopData() filename est seulement utilisé dans l'un des ifs. Dans touchFile la plupart de ces variables doivent être définies dans le try {} puisque vous ne les référencez pas dans le catch.

Généralement attraper le type Exception n'est pas une bonne idée. Cela peut mener à un comportement plutôt bizarre, même si cela peut avoir un sens ici. Attrapez les exceptions dont vous savez que vous pouvez gérer les autres bulles dans l'exécution.

Je voudrais aussi réfléchir à refactoriser cela en quelques classes. En considérant l'utilisation d'un modèle MVC-esque pour découpler le code, vous pouvez définir à quoi ressemble votre panneau et comment il réagit aux événements utilisateur/système de fichiers.

+0

Merci! Tout cela est génial. J'ai dû chercher MVC et c'était juste ce que je pensais dans mon esprit. Je voulais partitionner le code de manière sensée, mais je ne savais pas très bien quelle serait la meilleure approche. Merci! – zincc