2016-07-29 10 views
2

Zuvor hatte ich eine Methode namens "Search". Diese Methode hat tatsächlich zwei Dinge getan: Gesucht/gab die gefundenen Artikel zurück und es berechnete einige Daten des Resultsets. Ich habe jetzt zwei Methoden entwickelt: eine zum Suchen und Zurückgeben der Artikel. Die andere Sucht UND berechnet die Daten.Refactoring: Kombination von zwei Methoden

Die erste Methode, die nur die Suche durchführt:

private List<SummaryRootEntity> search(int contentId,SearchModel query, bool includeTypeGroupFilters = true) 
{ 
    cleanQuery(query); 

    var inactiveForDays = getinactiveForDays(); 

    var searchProfileGroups = _manager.GetSearchProfileGroupsForSite(query.PropertyType); 
    setLocationOfInterest(query); 
    var searchBounds = _manager.GetSearchBounds(query.StreetID, query.SublocalityID); 

    var shopsList = getShops(); 

    searchmodelDto.shops = shopsList; 
    var filteredArticles = _articleService.Search(
     contentId, 
     query, 
     searchBounds, 
     searchProfileGroups, 
     inactiveForDays, 
     includeTypeGroupFilters); 

    // apply ordering 
    var result = filteredArticles.ApplyOrdering(query.ForSaleOrRent, query.OrderBy, query.OrderDescending).ToList(); 

    return result; 
} 

Die zweite, die Statistiken sucht und berechnet:

private List<SummaryRootEntity> searchWithStats(
    int contentId, 
    SearchModel query, 
    out FacetStatisticsModel stats, 
    bool includeTypeGroupFilters = true) 
{ 
    cleanQuery(query); 

    var inactiveForDays = getinactiveForDays(); 

    var searchProfileGroups = _manager.GetSearchProfileGroupsForSite(query.Type); 
    setLocationOfInterest(query); 
    var searchBounds = _manager.GetSearchBounds(query.StreetID, query.SublocalityID); 

    var shopsList = getShops(); 

    query.shops = shopsList; 
    var filteredArticles = _articleService.Search(
     contentId, 
     query, 
     searchBounds, 
     searchProfileGroups, 
     inactiveForDays, 
     includeTypeGroupFilters); 

    var displaySearchResultSummary = getDisplaySearchResult ?? true; 
    var stats = _articleService.GetStatistics(
     Site.ContentRoot.Id, 
     shopsList, 
     inactiveForDays, 
     displaySearchResultSummary, 
     searchProfileGroups, 
     filteredArticles, 
     query, 
     searchBounds, 
     Site.DefaultCultureInfo); 

    // apply ordering 
    var result = filteredArticles.ApplyOrdering(query.ForSaleOrRent, query.OrderBy, query.OrderDescending).ToList(); 

    return result; 
} 

Der Beginn des zweiten Verfahrens die gleiche wie die erste. Die zweite Methode gibt Statistiken über den Parameter "out" zurück.

Gibt es eine Möglichkeit, die Berechnung von Statistiken in einer anderen Methode zu erhalten? Ich könnte die Suche nur aus der zweiten Methode herausreißen, aber dann müsste ich alle Variablen zweimal definieren (inactivForDays, searchProfileGroups, searchBounds).

+0

Ihre zweite Funktion füllt nie den out-Parameter, soweit ich sehen kann. – Nyerguds

+0

Es tut jetzt, Typo – user2939331

Antwort

2
public Dictionry<string, object> PrepareQueryValues(SearchModel query) 
{ 
    cleanQuery(query); 

    Dictionary<string, object> dic = new Dictionary<string, object>(); 

    dic.Add("InactiveForDay", getinactiveForDays()); 
    dic.Add("SearchProfileGroups",_manager.GetSearchProfileGroupsForSite(query.PropertyType)); 
    dic.Add("SearchProfileGroups", setLocationOfInterest(query)); 
    dic.Add("SearchBounds", _manager.GetSearchBounds(query.StreetID, query.SublocalityID)); 
    dic.Add("ShopsList", getShops()); 

    return dic;   
} 

So Ihre erste Methode:

private List<SummaryRootEntity> search(int contentId,SearchModel query, bool includeTypeGroupFilters = true) 
{ 
     Dictionary<string, object> dic = PrepareQueryValues(query); 

     searchmodelDto.shops = (CastToProperType)dic["ShopsList"]; 
     var filteredArticles = _articleService.Search(
     contentId, 
     query, 
     (CastToProperType)dic["SearchBounds"], 
     (CastToProperType)dic["SearchProfileGroups"], 
     (CastToProperType)dic["InactiveForDays"], 
     includeTypeGroupFilters); 

     // apply ordering 
     var result = filteredArticles.ApplyOrdering(query.ForSaleOrRent, query.OrderBy, query.OrderDescending).ToList(); 

     return result; 

} 

wie dieses Etwas sollte helfen, aber ich denke, das ist übertrieben.

+0

Wenn Sie den "Datenübertragungsobjekt" Weg sowieso gehen, ist es viel sauberer, nur eine tatsächliche Klasse dafür zu machen, anstatt ein Wörterbuch mit hardcoded Strings zu verwenden. – Nyerguds

+0

@Nyerguds Ich stimme dir zu 100% zu. – mybirthname

+0

Nicht super familiair mit Refactoring in .NET. Ist diese saubere Lösung sauber, wenn ich die Klassen anstelle der Wörterbücher verwende? – user2939331

0

Verwenden Sie einfach den Rückgabewert der Suche als Argument für die Statistik:

die Signaturen wird also wie folgt aussehen:

private List<SummaryRootEntity> search(
    int contentId, 
    SearchModel query, 
    bool includeTypeGroupFilters = true) 
{ 
} 

und

private FacetStatisticsModel searchWithStats(
    List<SummaryRootEntity> searchResults) 
{ 
} 

Das können Sie anrufen

var searchResult = Search(x, y, z); 

Oder

var searchResult = Search(x, y, z); 
var statsResult = Stats(searchResult); 
+0

Ich dachte auch, aber das würde bedeuten, dass beide Methoden die ersten ~ 7 Zeilen enthalten würde. Das wäre doppelter Code. Daher würden die Variablen (inactivForDays, searchProfileGroups, searchBounds) in Search() und in Stats() deklariert werden. – user2939331

0

Beide Funktionen machen das gleiche, nur einer von ihnen tut ein wenig mehr, oder?

Ich würde sie beide in eine Funktion machen, und einen zusätzlichen Parameter Boolean withStats zu den Funktionsargumenten hinzufügen. Dann können Sie einfach if/else in der Funktion verwenden, um die für jeden Fall erforderlichen Schritte auszuführen. Sie haben keinen doppelten Code wie diesen, nur alle Fälle in einer Funktion mit teilweise verzweigter Funktionalität.

Sie verlassen nun die tatsächliche volle Funktion erhalten, mit all seinen Argumenten wie:

private List<SummaryRootEntity> search(
    Int32 contentId, 
    SearchModel query, 
    Boolean withStats, 
    out FacetStatisticsModel stats, 
    Boolean includeTypeGroupFilters = true) 
{ 
    // I'll leave the actual combining part here up to you. 
    // Shouldn't be too hard to implement. 
} 

Nun, das ist ein bisschen klobig, sowohl mit dem Out-Parameter und der boolean; Ihre normalen Suchaufrufe müssten einen "out" -Parameter definieren, den sie niemals tatsächlich verwenden, und beide müssen den booleschen Wert angeben, der technisch davon abgeleitet werden kann, ob Sie diesen "out" -Parameter geben oder nicht geben wollen. Sie werden wahrscheinlich bevorzugen, dass die Anrufe so sind, wie sie vorher waren.

Nun, das ist einfach; mache einfach zwei Überladungen, die auf dieselbe zentrale Funktion zeigen, die du gerade gemacht hast. Das kümmert sich um diese Parameter für Sie.

Überlastung für die Suche mit Statistiken zu nennen:

private List<SummaryRootEntity> search(
    Int32 contentId, 
    SearchModel query, 
    out FacetStatisticsModel stats, 
    Boolean includeTypeGroupFilters = true) 
{ 
    return search(contentId, query, true, out stats, includeTypeGroupFilters); 
} 

Überlastung ohne Statistik für die Suche zu rufen, was macht und verwirft vollständig ein Dummy 'out' Parameter:

private List<SummaryRootEntity> search(
    Int32 contentId, 
    SearchModel query, 
    Boolean includeTypeGroupFilters = true) 
{ 
    FacetStatisticsModel stats; 
    return search(contentId, query, false, out stats, includeTypeGroupFilters); 
} 

Und Boom, Code verdichtet zu einer Funktion. Sie müssen natürlich sicherstellen, dass die Ausführung "withStats = false" den out-Parameter mit null ausfüllt.

Verwandte Themen