2009-04-13 6 views
1

J'ai écrit une classe d'aide qui prend une chaîne dans le constructeur et fournit beaucoup de propriétés Get pour retourner divers aspects de la chaîne. Actuellement, le seul moyen de définir la ligne est le constructeur et une fois qu'il est défini, il ne peut pas être modifié. Puisque cette classe a seulement une variable interne (la chaîne) je me demandais si je devrais le garder de cette façon ou devrais-je permettre à la chaîne d'être réglée aussi bien?Créer une nouvelle instance ou simplement définir des variables internes

Quelques exemples de code mon aide pourquoi je demande:

StreamReader stream = new StreamReader("ScannedFile.dat"); 
ScannerLine line = null; 
int responses = 0; 
while (!stream.EndOfStream) 
{ 
    line = new ScannerLine(stream.ReadLine()); 
    if (line.IsValid && !line.IsKey && line.HasResponses) 
    responses++; 
} 

Au-dessus est un exemple rapide de compter le nombre de réponses valides dans un fichier numérisé donné. Serait-il plus avantageux de le coder comme ça à la place?

StreamReader stream = new StreamReader("ScannedFile.dat"); 
ScannerLine line = new ScannerLine(); 
int responses = 0; 
while (!stream.EndOfStream) 
{ 
    line.RawLine = stream.ReadLine(); 
    if (line.IsValid && !line.IsKey && line.HasResponses) 
    responses++; 
} 

Ce code est utilisé dans l'extrémité arrière d'une application Web ASP.net et doit être assez réactif. Je suis conscient que cela peut être un cas d'optimisation prématurée mais je suis en train de coder ceci pour la réactivité du côté client et la maintenabilité.

Merci!

EDIT - j'ai décidé d'inclure le constructeur de la classe ainsi:

public class ScannerLine 
{ 
    private string line; 
    public ScannerLine(string line) 
    { 
    this.line = line; 
    } 

    /// <summary>Gets the date the exam was scanned.</summary> 
    public DateTime ScanDate 
    { 
    get 
    { 
     DateTime test = DateTime.MinValue; 
     DateTime.TryParseExact(line.Substring(12, 6).Trim(), "MMddyy", CultureInfo.InvariantCulture, DateTimeStyles.None, out test); 
     return test; 
    } 
    } 

    /// <summary>Gets a value indicating whether to use raw scoring.</summary> 
    public bool UseRaw { get { return (line.Substring(112, 1) == "R" ? true : false); } } 

    /// <summary>Gets the raw points per question.</summary> 
    public float RawPoints 
    { 
    get 
    { 
     float test = float.MinValue; 
     float.TryParse(line.Substring(113, 4).Insert(2, "."), out test); 
     return test; 
    } 
    } 
} 

** EDIT 2 - ** I inclus quelques propriétés de l'échantillon (Oui, c'est ce qu'il est vraiment.) de la classe pour aider à clarifier. Comme vous pouvez le voir, la classe prend une chaîne fixe d'un scanner et facilite simplement la séparation de la ligne en morceaux plus utiles. Le fichier est un fichier délimité par une ligne à partir d'une machine Scantron et la seule façon de l'analyser est un groupe de chaînes. Appels et conversions de sous-chaînes.

+0

Vous devez conclure ce StreamReader dans une instruction using. – RossFabricant

+0

Haha, j'ai effectivement une fermeture explicite après que c'est fait, mais j'ai nettoyé le code pour se concentrer sur le problème. – Joshua

Répondre

4

Je voudrais vraiment rester avec la version immuable si vous avez vraiment besoin de la classe du tout. L'immutabilité facilite le raisonnement sur votre code - si vous stockez une référence à un ScannerLine, sachez que cela ne va pas changer. La performance est presque certaine d'être insignifiante - l'IO impliqué dans la lecture de la ligne est susceptible d'être plus important que de créer un nouvel objet. Si vous êtes vraiment préoccupé par les performances, vous devriez devrait comparer/profiler le code avant de décider de prendre une décision de conception basée sur ces soucis de performance.

Cependant, si votre état est juste une chaîne, fournissez-vous vraiment beaucoup d'avantages en stockant simplement les chaînes directement et en ayant des méthodes appropriées pour les analyser plus tard? Est-ce que ScannerLine analyse la chaîne et cache cette analyse, ou est-ce vraiment juste un tas de méthodes d'analyse?

+0

En interne, il contient un ensemble de propriétés get qui analysent la chaîne dans divers formats et effectue des conversions et autres. Il a été écrit pour être réutilisable dans ce cas pour séparer rapidement la chaîne. – Joshua

+0

Mais ne peuvent-ils pas être des méthodes statiques qui prennent un argument de chaîne? Je ne dis pas que c'est forcément une meilleure idée, mais cela semble plus simple. Si vous utilisez C# 3.0, vous pouvez en faire des méthodes d'extension. –

+0

Actuellement, je code vers C# 2.0. J'ai mis à jour la classe pour aider à clarifier à quoi elle sert. – Joshua

0

Votre première approche est plus claire. Performance sage, vous pouvez gagner quelque chose, mais je ne pense pas que vaut la peine.

0

J'irais avec la deuxième option. C'est plus efficace, et ils sont tous deux faciles à comprendre. De plus, vous n'avez probablement aucun moyen de savoir combien de fois ces instructions seront appelées dans la boucle while. Alors qui sait? Il pourrait s'agir d'un gain de performance de 0,01%, ou d'un gain de performance de 50% (peu probable, mais peut-être)!

0

Les classes immuables ont beaucoup d'avantages. Il est logique qu'une classe de valeur simple comme celle-ci soit immuable. L'heure de création des objets pour les classes est faible pour les machines virtuelles modernes. La façon dont vous l'avez est très bien.

0

Je supprimerais complètement la nature "instance" de la classe et l'utiliserais comme une classe statique, pas une instance comme vous l'êtes actuellement.Chaque propriété est entièrement indépendante l'une de l'autre, SAUF pour la chaîne utilisée. Si ces propriétés étaient liées entre elles, et/ou si d'autres variables "cachées" étaient configurées à chaque fois que la chaîne était affectée (pré-traitement des propriétés par exemple), alors il y aurait des raisons de le faire façon ou l'autre avec réaffectation, mais de ce que vous faites là, je le changerais pour être 100% des méthodes statiques de la classe. Si vous insistez pour que la classe soit une instance, alors pour des raisons de performances pures, j'autorise la réaffectation de la chaîne, car alors le CLR ne crée et ne détruit pas continuellement les instances de la même classe (sauf pour la chaîne elle-même évidemment).

A la fin de la journée, l'OMI est ce quelque chose que vous pouvez vraiment faire quelque façon que vous voulez car il n'y a pas d'autres variables d'instance de classe. Il peut y avoir des raisons de style pour faire l'un ou l'autre, mais il serait difficile d'avoir tort quand on résout ce problème. S'il y avait d'autres variables dans la classe qui étaient définies lors de la construction, alors ceci serait un problème totalement différent, mais maintenant, codez pour ce que vous voyez comme le plus clair.

+0

Je suis d'accord avec tout sauf la partie "performance pure". Les chances sont que la lecture de la chaîne prendra plus de temps en premier lieu - la création d'un objet est très bon marché dans .NET, et l'immuabilité * est * une belle propriété d'un type. Mais oui, encapsuler juste une chaîne semble un peu inutile. –

0

je partirais avec votre première option. Il n'y a aucune raison que la classe soit mutable dans votre exemple. Restez simple à moins que vous ayez réellement besoin de le rendre mutable. Si vous êtes vraiment préoccupé par les performances, effectuez des tests d'analyse des performances et observez les différences.

Questions connexes