2010-09-21 27 views
0

Pour mon projet de structures de données, le but est de lire dans un fichier fourni contenant plus de 10000 chansons avec l'artiste, le titre et les paroles clairement marqués, et chaque chanson est séparée par une ligne avec un double guillemet. J'ai écrit ce code pour analyser le fichier texte, et il fonctionne, avec une durée d'un peu moins de 3 secondes pour
lire les 422k lignes de texte
créer un objet Song
ajouter ledit morceau à un ArrayList
De quelle autre manière ce code peut-il être optimisé pour la programmation défensive?

le code d'analyse syntaxique je l'ai écrit est:

if (songSource.canRead()) { //checks to see if file is valid to read 
    readIn= new Scanner(songSource); 
    while (readIn.hasNextLine()) { 
do { 
    readToken= readIn.nextLine(); 

      if (readToken.startsWith("ARTIST=\"")) { 
    artist= readToken.split("\"")[1]; 
     } 
     if (readToken.startsWith("TITLE=\"")) { 
    title= readToken.split("\"")[1]; 
     } 
     if (readToken.startsWith("LYRICS=\"")) { 
    lyrics= readToken.split("\"")[1]; 
     } else { 
    lyrics+= "\n"+readToken; 
     }//end individual song if block 
} while (!readToken.startsWith("\"")); //end inner while loop 

    songList.add(new Song(artist, title, lyrics)); 

    }//end while not EOF 
} //end if file can be read 

Je discutais avec mon introduction au professeur Algorithmes sur le code pour ce projet, et il a déclaré que je devrais essayer d'être plus défensif dans mon code pour permettre incohérences dans les données fournies par d'autres personnes. À l'origine, j'utilisais des blocs if/else entre les champs Artist, Title et Lyrics, et sur sa suggestion je passais à des instructions if séquentielles. Bien que je puisse voir son point de vue, en utilisant cet exemple de code, comment puis-je être plus défensive en ce qui concerne les incohérences de saisie?

+0

C'est une question ouverte pour rendre votre code "meilleur". Envisagez-vous d'en faire un wiki communautaire? – pascal

Répondre

2

Vous supposez que l'entrée est parfaite. Si vous regardez la façon dont votre application est actuellement installée, la base d'une lecture rapide de votre algorithme, les données ressembleraient cette

ARTIST="John" 
TITLE="HELLO WORLD" 
LYRICS="Sing Song All night long" 
" 

Mais considérez le cas

ARTIST="John" 
TITLE="HELLO WORLD" 
LYRICS="Sing Song All night long" 
" 
ARTIST="Peter" 
LYRICS="Sing Song All night long" 
" 

Sur la base de votre algorithme, vous ont maintenant 2 chansons caractérisé en

songList = { Song("JOHN", "HELLO WORLD", "Sing Song All night long"), 
      Song("Peter", "HELLO WORLD", "Sing Song All night long") } 

Avec l'algorithme actuel, l'artiste et le titre sont exposés et apparaîtront dans la 2ème chanson, même si elles ne sont pas définis. Vous devez réinitialiser vos trois variables.

dans votre autre, vous êtes juste en train de jeter la ligne complète dans les paroles. Et si vous aviez déjà retiré les paroles, vous êtes en train de passer outre. Cas de test

ARTIST="John" 
LYRICS="Sing Song All night long" 
TILET="HELLO WORLD" 
" 

Envisagez d'envoyer cet enregistrement à un état d'erreur. Ainsi, lorsque la lecture du lot est terminée, un rapport d'erreur peut être généré et corrigé.

En outre, vous considérez uniquement EOF après la lecture d'un artiste. Que se passe-t-il si l'EOF survient pendant la lecture de l'artiste, et le fichier ne se termine pas en "Vous allez recevoir une exception. un autre chèque de hasNextLine()

+0

J'ai choisi votre réponse comme réponse, car même si je ne l'utilisais pas dans le projet actuel, vous m'avez donné un exemple concret pour contourner les erreurs de saisie des utilisateurs sans écraser un programme. – Jason

4

Je remplacerais par exemple:

artist= readToken.split("\"")[1]; 

avec

String[] parts = readToken.split("\""); 
if(parts.length >= 2) artist = parts[1]; 
else continue; 

D'autres modifications comprennent:

  1. réinitialiser les variables locales (afin de ne pas accidentellement le mauvais artiste pour une chanson, si aucun artiste n'est fourni pour une chanson après la première)
  2. décidez quoi faire s'il manque des données - voulez-vous toujours ajouter la chanson à la liste des chansons?
+0

L'expression rationnelle peut-elle être optimisée ou est-elle construite pour chaque itération? –

+0

@Thorborn - cela n'affecte pas la robustesse du code ... seulement ses performances. Et même alors, pas beaucoup. (IIRC, la méthode Pattern.compile utilise un simple cache à un regex pour éviter de compiler plusieurs fois la même regex, dans ce cas, ce serait efficace.) –

+0

Merci pour l'exemple, vous m'avez donné quelques idées à faire. – Jason

2

Dans le monde réel, il y a des garanties en ce qui concerne l'intégrité des données. Dans le cas d'une saisie par l'utilisateur (que ce soit à partir de stdin ou d'un fichier), il existe un paradigme défini par le projet pour informer l'utilisateur d'un problème nécessitant une attention particulière. Par exemple, quand un compilateur compilant du code ou un shell exécutant un script rencontre une incohérence il peut arrêter et imprimer la ligne contenant l'incohérence avec une deuxième ligne en dessous qui utilise le symbole "^" pour indiquer l'emplacement de la problème.

Voici donc quelques questions de base à vous poser:
1. Chaque ligne est-elle garantie pour contenir tous les champs?
2. La commande des champs est-elle garantie?

Si ce sont les conditions du contrat d'entrée et sont violés, vous devez ignorer/signaler la ligne. Si elles ne sont pas des conditions de l'entrée, alors vous devez le gérer .. ce que vous ne faites pas actuellement.

+0

Ce genre de chose est exactement pourquoi j'aime ce site. Je pense que je rencontre certaines des lacunes de la façon dont ce programme CS est conçu. Il semble plus intéressant de commencer par les concepts globaux, puis d'entrer dans les détails, mais à part un document sur la syntaxe du code et le blanc, il y a très peu d'accent sur les procédures appropriées comme le codage défensif. – Jason

0

Voici quelques questions qui pourraient être abordées:

  • Votre code suppose qu'il n'y a pas d'espace avant (par exemple) « ARTISTE », aucun autour du signe « = » et ainsi de suite.

  • Votre code suppose que les mots-clés sont en majuscules. Quelqu'un pourrait utiliser des minuscules ou des cas mixtes.

  • Votre code suppose qu'une ligne qui ne commence pas par keyword=\" est la suite des paroles de la chanson. Mais que faire si l'utilisateur a entré ARTOST="Sting"? Ou que faire si l'utilisateur a essayé d'utiliser deux lignes pour un nom d'artiste?

Enfin, je ne suis pas convaincu que le remplacement de « autre si » par « si » dans ce cas a fait une différence dans la robustesse du code.

1

Je vois un certain nombre de choses qui manquent ici Jason.

Je pense que le if/else était bon et il ne changera pas la logique. Cependant, vous devriez limiter autant que possible la portée de vos variables. En déclarant artiste, titre, etc. à l'intérieur de la boucle while, ils seront initialisés à null (ou quelque chose d'autre), donc si une entrée manque à l'artiste, elle n'obtiendra pas la valeur de la dernière entrée.

En outre, que se passe-t-il si un titre, un artiste, etc. a une citation? Comment cela est-il géré? Que diriez-vous des paroles qui semblent être plusieurs lignes à droite? Que se passe-t-il s'il y a un champ inconnu - peut-être une faute d'orthographe? Il sera ajouté à la fin des paroles qui ne semble pas correct. Une fois que le champ LYRICS a été trouvé, vous devez l'ajouter. Si les paroles sont nulles, elles commenceront par "null".

0

Traiter avec des exceptions (je suppose Scanner pourrait jeter InputMismatchException pour un caractère non valide).

Il ressemble à la boucle do { } while (...) de boîte sans fin si le fichier est mal formé, et la fin de le fichier est atteint

Rien n'empêche artist ou title d'être vide.