Wie diese Klasse/Methode aktualisieren Codemetrik

Question

Dies war meine erste Methode

public String GetAllDocuments(string url,int pager =0) 
    { 
     if (SessionInfo.IsAdmin) 
     { 
      ReportHandler dal = new ReportHandler(); 
      var documents = dal.FetchDocumentsList(SessionInfo.ClientID, pager); 
      string documentsDataJSON = JsonConvert.SerializeObject(documents); 

      return documentsDataJSON; 
     } 
     else 
     { 
      return "Sorry!! You are not authorized to perform this action"; 
     } 
    } 

Visual Studio zeigt die folgenden Code-Metriken zu verbessern: -

Mitglied: GetAllDocuments (string, int): String
Wartbarkeit Index: 67
Zyklomatische Komplexität: 2
Klasse Kupplung: 7
Codezeilen: 7

Also, um es zu verbessern, modifizierte ich meine Methode wie folgt: -

public String GetAllDocuments(string url,int pager =0) 
    { 
      ReportHandler dal = new ReportHandler(); 
      var documents = dal.FetchDocumentsList(SessionInfo.ClientID, pager); 
      //moved the JSON Conversion to Separate class 
      string documentsDataJSON = JsonHandler<T>.ConvertToJSON(documents); 
      return documentsDataJSON; 

    } 

Aber noch es zeigt die Codemetrik als

Mitglied: GetAllDocuments (string, int) : String
Wartbarkeitsindex: 72
Cyclomatic Komplexität: 1
Klassenkopplung: 5
Codezeilen: 5

Ich kann dies nicht einreichen, außer der Maintainability Index ist 90+.

Was kann ich noch tun, um Code Metrics zu verbessern?

Außerdem bin ich für solche kleinen Dinge zu schaffen separate Methode/Klasse denken, ist es nicht Overhead

Answer 1

Wenn Sie Interesse an gerade lesen Wartbarkeit Index besser zu machen, wie diese berechnet wird: http://www.projectcodemeter.com/cost_estimation/help/GL_maintainability.htm

Sie kann sehen, dass es hauptsächlich von der Anzahl der Codezeilen abhängt.

Wenn Sie ein gutes Design wünschen, verwenden Sie keine konkreten Implementierungen, verwenden Sie einen der SOLID-Prinzipien - Dependency Inversion. Zum Beispiel, anstelle der Verwendung:

ReportHandler dal = new ReportHandler(); 

shoud Sie so etwas wie verwenden:

IReportHandler dal = reportHandlerFactory.GetReportHandler(); 
// where reportHandlerFactory is IReportHandlerFactory which is also 
// make dependency on interface but not on concrete class 

Oder noch besser ist, solche Dinge mit DI Behälter zu injizieren.

Answer 2

Sie sollten häufig die Erstellung von Objekten in einer Factory-Schnittstelle kapseln, die Sie an den Konstruktor der Klasse übergeben, die sie verwenden möchte.

So Ihre Klasse würde einen Konstruktor und Feld wie so haben:

public MyClass(IReportHandlerFactory reportHandlerFactory) 
{ 
    if (reportHandlerFactory == null) 
     throw new ArgumentNullException(nameof(reportHandlerFactory)); 

    _reportHandlerFactory = reportHandlerFactory; 
} 

private readonly IReportHandlerFactory _reportHandlerFactory; 

Dies als Dependency Injection bekannt ist, speziell Injektion Konstruktor.

Um dies effektiv zu tun, würden Sie wahrscheinlich auch eine Schnittstelle für Ihre ReportHandler Klasse erstellen möchten.Dann würde die Fabrik Schnittstelle etwas so:

public interface IReportHandlerFactory 
{ 
    IReportHandler Create(); 
} 

Und Bericht Handler-Schnittstelle wie folgt aus:

public interface IReportHandler 
{ 
    IEnumerable<Document> FetchDocumentsList(Guid clientID, int pager); 
} 

... und so weiter. Hoffentlich kommst du auf die Idee.

Sie sollten auch Ihre Methode aufgeteilt, vielleicht so etwas wie folgt aus:

public String GetAllDocuments(string url,int pager =0) 
{ 
    if (SessionInfo.IsAdmin) 
    { 
     return documentsData(_reportHandlerFactory, SessionInfo.ClientID, page); 
    } 
    else 
    { 
     return "Sorry!! You are not authorized to perform this action"; 
    } 
} 

private static String documentsData(
    IReportHandlerFactory reportHandlerFactory, 
    Guid clientID, 
    int pager) 
{ 
    IReportHandler dal = reportHandlerFactory.Create(); 

    var documents = dal.FetchDocumentsList(clientID, pager); 
    string documentsDataJSON = JsonConvert.SerializeObject(documents); 

    return documentsDataJSON; 
} 

HINWEIS: Um ehrlich zu sein, diese Frage wirklich eine bessere Passform ist für https://codereview.stackexchange.com/

ich auf die folgenden Bücher empfehlen zu diesem Thema:

Clean Code: A Handbook of Agile Software Craftsmanship (Robert C. Martin)

und

Dependency Injection in .NET (Mark Seemann)

oder Mark Seemann Blog.

Answer 3

Nehmen Sie einen Wartbarkeitsindex als Werkzeug, kein Ziel.

Es hat keinen Sinn, den Wartbarkeitsindex zu erhöhen, wenn Sie nicht erkennen, warum Ihr Code überhaupt nicht gewartet werden konnte. Sie werden den Code immer weiter verschieben, um den Index zu erfüllen, ohne jemals zu verstehen, was Sie tun.

Ihre Frage sollte nicht sein

Wie diese Klasse/Methode aktualisieren-Metriken zu verbessern?

aber

Wie die Wartbarkeit dieser Klasse verbessern?

Ich sehe eine Reihe von Problemen mit sich sofort:

• Abhängigkeiten sind implizit, das heißt Sie new ‚sind Dinge ing direkt innerhalb der Methode auf. Dies macht den Code weniger flexibel, zusammensetzbar und lesbar.

  Die Übergabe der ReportHandler-Abhängigkeit an GetAllDocuments() oder den Klassenkonstruktor wäre stattdessen besser.

• Es ist schlecht testbar. Wenn ReportHandler stattdessen eine Schnittstelle (oder eine abstrakte Klasse) wäre, könnten Sie sie in Tests für GetAllDocuments() für eine verbesserte Leistung und Tests durch einen falschen Report-Handler ersetzen. Beachten Sie, dass Sie keine Factory verwenden müssen, um dies zu tun, eine einfache Schnittstelle mit einer realen Implementierung und einer Testimplementierung sind gut genug.

• Der Parameter url wird nicht verwendet.

• SessionInfo.IsAdmin ist eine Art magischer Kurzschrift, die für die gleiche Reihe von Problemen wie die oben genannten anfällig ist. Wenn Sie in einem Controller sind, ist es keine große Sache, aber wenn Sie in einer Business-Schicht sein sollen, wird dies Testbarkeit und Wartbarkeit behindern.