[C#] Best practice, Extension method of normale method

[HMS]

Tijdens het bouwen van mijn applicatie (muziek collectie dingetje, prive projectje ) kwam ik op het idee om extension methods te (mis?)(ge)bruiken. Aangezien er veel met directories en bestanden gewerkt wordt zijn extension methods vrij handig om de code leesbaar te houden.

Voor wie onbekend is met extension methods: http://msdn.microsoft.com/en-us/library/bb383977.aspx

Nu zijn er natuurlijk altijd 2 (of meer?) mogelijkheden om problemen op te lossen, nu gaat het mij om het ophalen van files uit een directory.

Dus, extension method:

C#:

public static FileInfo GetSfvFile(this DirectoryInfo directory) {
  return directory.EnumerateFiles("*.sfv").SingleOrDefault(); //Neem aan dat er maar 1 SFV file in een directory zit.
}

of normale method:

C#:

public FileInfo GetSfvFile(DirectoryInfo directory) {
 //body zelfde als extension method.
}

Het grote verschil tussen deze twee is de aanroep:

C#:

public void foo() {
  //Assume: var directory = DirectoryInfo instance
  var file = directory.GetSfvFile();
}

of:

C#:

public void foo() {
  //Assume: var directory = DirectoryInfo instance
  var file = GetSfvFile(directory);
}

Wat heeft jullie voorkeur? En zijn er nog voor / nadelen aan bepaalde methodes die ik over het hoofd heb gezien?

Als er een betere manier is, dan hoor ik dat natuurlijk graag .

[pedorus]

Dit ziet er uit als een methode van 1 regel die je maar 1x gebruikt, dus ik zou helemaal geen methode ervoor maken..

Verder is dit niet echt een algemene library-achtige methode, dus ligt een extention method niet voor de hand. Mocht MS ooit een methode GetSfvFile toevoegen aan DirectoryInfo, dan zal de extention method breken. Daarnaast vervuilt het je namespace maar.

Uit de guidelines:

quote: http://msdn.microsoft.com/en-us/library/bb383977.aspx

In general, we recommend that you implement extension methods sparingly and only when you have to. Whenever possible, client code that must extend an existing type should do so by creating a new type derived from the existing type. For more information, see Inheritance (C# Programming Guide).

When using an extension method to extend a type whose source code you cannot change, you run the risk that a change in the implementation of the type will cause your extension method to break.

If you do implement extension methods for a given type, remember the following two points:

• An extension method will never be called if it has the same signature as a method defined in the type.
• Extension methods are brought into scope at the namespace level. For example, if you have multiple static classes that contain extension methods in a single namespace named Extensions, they will all be brought into scope by the using Extensions; directive.

[RobIII]

pedorus schreef op woensdag 18 augustus 2010 @ 01:22:
Mocht MS ooit een methode GetSfvFile toevoegen aan DirectoryInfo, dan zal de extention method breken.

Met die redenatie kun je natuurlijk meteen helemaal de extension methods afschaffen _{Los van het feit dat ik de kans klein acht}. Maar ondanks dat ben ik het wel met je eens; ik zie/bespeur hier geen goed argument om een extension method te gebruiken anders dan "het is zo hip"

[HMS]

Dus het grote nadeel is (alleen ?):

When using an extension method to extend a type whose source code you cannot change, you run the risk that a change in the implementation of the type will cause your extension method to break.

Maar wat is de afweging om een extension te schrijven of een aparte methode te implementeren? Dat is mij nog niet helemaal duidelijk.

[Killemov]

Voor dit specifieke voorbeeld zou ik de regel code inlinen. Je maakt geen enkele verbetering in de code met het wegstoppen ervan.
Verder is de extension methode het meest object-georienteerd. Je vraagt iets uit het "binnenste" van een folder, typische OO-oplossing dus.

[RobIII]

HMS schreef op woensdag 18 augustus 2010 @ 01:45:
Maar wat is de afweging om een extension te schrijven of een aparte methode te implementeren? Dat is mij nog niet helemaal duidelijk.

Bekijk dit artikel (PDF) eens; schoot me net te binnen dat het een tijdje geleden in .Net Magazine had gestaan en dat was wel een aardig artikeltje imho.

Persoonlijk vind ik GetSfvFile geen verantwoordelijkheid van de DirectoryInfo klasse maar van 1 van je eigen klassen. Dat je die eventueel afleidt van DirectoryInfo is wel een mogelijkheid; afhankelijk natuurlijk wat je nog meer van plant bent ermee dan. Het kan wel wat je wil, maar het is niet de meest logische plaats (imho) op die methode te plaatsen.

[ Voor 57% gewijzigd door RobIII op 18-08-2010 02:16 ]

[pedorus]

RobIII schreef op woensdag 18 augustus 2010 @ 01:31:
[...]

Met die redenatie kun je natuurlijk meteen helemaal de extension methods afschaffen

Klopt in zekere zin. Maar de gekozen naam is niet zodanig dat er maar 1 logische implementatie mogelijk is, en er zijn ook geen parameters die het verschil kunnen maken.

_{Los van het feit dat ik de kans klein acht}

Ik acht de kans zelfs verwaarloosbaar.

HMS schreef op woensdag 18 augustus 2010 @ 01:45:
Dus het grote nadeel is (alleen ?):

Nee het nadeel is onnodige namespacevervuiling, en waarschijnlijk onnodige onduidelijkheid en verspreiding van code.

Nog een extra redenatie: Extention methods gebruik je meestal alleen als je meerdere classes die dezelfde interface implementeren wil extenden met eenzelfde functie (bijvoorbeeld IEnumerable<T> of ICollection<T>). Een goed voorbeeld van nuttig gebruik is bijvoorbeeld morelinq. Een ander voorbeeld van correct gebruik is een functie met een eenduidige uitleg die in de library zelf had kunnen zitten (zeg String.Reverse()). Hier extend je daarentegen slechts 1 class voor een project-specifieke functie.

Verder geld natuurlijk de vraag of een functie in een bepaalde class thuishoort ook deels voor extention methods. Voorbeeld: een extention method als [class].ShowOnScreen() is dus een zeer fout idee imo. Wat RobIII er net heeft bijgeedit dus.

[HMS]

Volgens mij begin ik het te snappen wat jullie bedoelen. Mogelijk heb ik ook een slecht voorbeeld gebruikt natuurlijk. Zelf heb ik nog niet met dusdanig grote namespaces gewerkt dat er echt sprake van vervuiling is / zou zijn.

Dat artikeltje is inderdaad wel handig RobIII, maar is meer een introductie van Extension methods. Hoewel op het eind wel een paar bad practices aangestipt worden, waar mijn eerste voorbeeld inderdaad ook bij hoort .

Zou dit een voorbeeld zijn van een goede keus voor een extension method? Vooral omdat het op meerdere plaatsen gebruikt wordt en makkelijker leest in if-statements.

C#:

        public static bool IsEmpty<TSource>(this IEnumerable<TSource> enumerable)
        {
            return enumerable.Count() == 0; //Extension method in extension method, misschien ook niet zo goed :P
        }

Snap zelf niet zo goed waarom er standaard geen .IsEmpty() extension is, maar goed.

[pedorus]

HMS schreef op woensdag 18 augustus 2010 @ 02:35:
Snap zelf niet zo goed waarom er standaard geen .IsEmpty() extension is, maar goed.

Die is er wel, hij staat hierboven. Ok, misschien niet standaard. Voor een discussie hierover, zie stackoverflow. Over het algemeen wil je toch iets doen als je elementen hebt, dus kun je Any() gebruiken.

[HMS]

pedorus schreef op woensdag 18 augustus 2010 @ 02:45:
[...]

Die is er wel, hij staat hierboven. Ok, misschien niet standaard. Voor een discussie hierover, zie stackoverflow. Over het algemeen wil je toch iets doen als je elementen hebt, dus kun je Any() gebruiken.

Haha juistem, het wordt al laat merk ik. Dank voor het SO linkje, weer wat wijzer geworden. Ik programmeer tegenwoordig te weinig merk ik al .

offtopic:
Vakantie helpt ook niet mee natuurlijk

[BurningSheep]

HMS schreef op woensdag 18 augustus 2010 @ 01:11:
Dus, extension method:

C#:

public static FileInfo GetSfvFile(this DirectoryInfo directory) {
  return directory.EnumerateFiles("*.sfv").SingleOrDefault(); //Neem aan dat er maar 1 SFV file in een directory zit.
}

of normale method:

C#:

public FileInfo GetSfvFile(DirectoryInfo directory) {
 //body zelfde als extension method.
}

Eigenlijk vind ik het allebei niet zo mooi. Ik zie vaak mensen die extension methods net hebben ontdekt ze te pas en te onpas toepassen. Ze maken de code moeilijk te doorgronden omdat er op willekeurige plekken methodes toegevoegd worden aan klassen. Die instance method vind ik niet mooi omdat GetSfvFile in dit geval helemaal niets met het object doet waarin hij gedefinieerd is. Ik zou daar een static helper methode van maken, of nog beter, er een nieuw object introduceren die de verantwoordelijkheid heeft om de lijst op te halen.