[C#/ASP.Net] Mail methode/functie in static class?

[Erwin_Br]

Hallo,

In mijn webapplicatie gebruik ik een aantal maal de mail functionaliteit. Mailen in .Net gaat vrij makkelijk, maar om deze eenvoudige code niet op elke pagina te zetten waarin ik wil mailen, heb ik een class aangemaakt met een functie die er als volgt uitziet:

code:

public class sendMailClass
{
    public static bool sendMail(string from, string to, string subject, string body)
    {
        try
        {
            MailMessage message = new MailMessage(from, to, subject, body);
            message.IsBodyHtml = true;
            SmtpClient emailClient = new SmtpClient();
            emailClient.Send(message);
            return true;
        }
        catch (Exception)
        {
            return false;
        }
    }
}

Het werkt prima, daar gaat het niet om. Ik vraag me echter af of dit de juiste en/of nette manier is.

Tevens vraag ik mij af wat de consequenties zijn van het gebruik van een static functie in dit geval. Statics hebben namelijk een application scope. Gaat dit wel goed als meerdere gebruikers op hetzelfde moment gebruikmaken van de mail functie?

Alvast bedankt.

--Erwin

[Janoz]

Of het helemaal nietjes is.... Ik doe zelf niks met .Net, maar ik krijg wel een gevoel dat er betere oplossingen zouden moeten zijn. Daarnaast is het natuurlijk behoorlijk smerig om de exception maar gewoon te negeren zonder ook maar enige vorm van logging.

Maar om op je laatste vraag terug te komen:

Tevens vraag ik mij af wat de consequenties zijn van het gebruik van een static functie in dit geval. Statics hebben namelijk een application scope. Gaat dit wel goed als meerdere gebruikers op hetzelfde moment gebruikmaken van de mail functie?

Dat moet volgens mij geen enkel probleem opleveren. Je gebruikt enkel locale variabelen dus je hebt geen (race) problemen wanneer er meerdere aanroepen tegelijk gebeuren.

[? ?]

..

[ Voor 115% gewijzigd door ? ? op 25-01-2013 09:43 ]

[creator1988]

Static class voor mail versturen is niets mis mee:

Wel met het naapen van MailMessage functionaliteit. Je functie heeft exact dezelfde parameters als een van de MailMessage constructors, waarmee je dus aan flexibiliteit inboet terwijl het geen duidelijk voordeel geeft.

Verder inderdaad usings, en try-catch afvangen en dan true/false teruggeven is vrij ranzig.

Een SmtpClient emailClient noemen vind ik zelf niet duidelijk, want het is geen emailClient maar een smptClient, maar dat is weer mierenneuken.

[Matis]

creator1988 schreef op dinsdag 28 april 2009 @ 08:54:
Verder inderdaad usings, en try-catch afvangen en dan true/false teruggeven is vrij ranzig.

Ben ik met je eens, ik persoonlijk zal true pas returnen buiten de try catch, maar dat is meer een manier van smaak.

Ook doe je een new() in je methode, ik weet niet precies hoe de garbagecollector werkt van C# / ASP, maar misschien moet je die resource weer vrijgeven.

Edit:

ik heb even nagekeken en volgensmij krijg je op deze manier een memory-leak:

C#:

public class Test {

public static HashSet set = new HashSet();
}

Test.set = null;

Nu wordt de recourse vrijgegeven voor garbage collection.

Bij jou zal er telkens een nieuwe instantie worden aangemaakt (geheugen gereserveerd) net zolang totdat de resource weer op null wordt gezet.

Ik raad je dus aan dit nog een keer te doen, anders is de kans groot dat je memory-leaks krijgt!

Zo dus:

C#:

public class sendMailClass
{
    public static bool sendMail(string from, string to, string subject, string body)
    {
        try
        {
            MailMessage message = new MailMessage(from, to, subject, body);
            message.IsBodyHtml = true;
            SmtpClient emailClient = new SmtpClient();
            emailClient.Send(message);
            emailClient = null;
            message = null;
        }
        catch (Exception)
        {
            return false;
        }
     return true; // Vind ik persoonlijk netter, maar mag ook in de try block!
    }
}

[ Voor 58% gewijzigd door Matis op 28-04-2009 09:09 ]

[Woy]

toaomatis schreef op dinsdag 28 april 2009 @ 09:00:
[...]
Bij jou zal er telkens een nieuwe instantie worden aangemaakt (geheugen gereserveerd) net zolang totdat de resource weer op null wordt gezet.

Ik raad je dus aan dit nog een keer te doen, anders is de kans groot dat je memory-leaks krijgt!

Zo dus:

C#:

public class sendMailClass
{
    public static bool sendMail(string from, string to, string subject, string body)
    {
        try
        {
            MailMessage message = new MailMessage(from, to, subject, body);
            message.IsBodyHtml = true;
            SmtpClient emailClient = new SmtpClient();
            emailClient.Send(message);
            emailClient = null;
            message = null;
        }
        catch (Exception)
        {
            return false;
        }
     return true; // Vind ik persoonlijk netter, maar mag ook in de try block!
    }
}

Dat is onzin, message op null assignen is totaal niet nodig. Aangezien de variabele gewoon uit scope gaat zal hij gewoon netjes opgeruimd worden ( al zal dat waarschijnlijk niet meteen zijn ), maar het op null zetten veranderd daar niks aan. Het op null zetten van de variabele ruimt het object overigens ook niet op.

In C++ is dat natuurlijk anders, waar je voor elke new een matchende delete moet hebben om je geheugen vrij te geven. Maar daar heb je ook Deterministic Destruction, en in C# weet je niet wanneer het object daadwerkelijk opgeruimd word. Dat is ook de hele reden van het using/IDisposable verhaal, zodat je al vroegtijdig resources kan vrijgeven.

[ Voor 11% gewijzigd door Woy op 28-04-2009 09:28 ]

[Haan]

De nette manier is volgens mij om het zo te doen: (wat Woy er dus al bij heeft ge-edit in de tussentijd )

C#:

       try
        {
            using (MailMessage message = new MailMessage(from, to, subject, body))
            {
                message.IsBodyHtml = true;
                SmtpClient emailClient = new SmtpClient()
                emailClient.Send(message);
            }
        } 

Ik gebruik ook een vergelijkbare static methode om email te verzenden in m'n eigen frameworkje, niets mis mee imho

[ Voor 5% gewijzigd door Haan op 28-04-2009 09:35 ]

[Woy]

Idd, het is ieder geval een goede gewoonte om op alle classes die IDisposable implementeren te Disposen na gebruik. Als je ze in een enkele scope gebruikt is dat het makkelijkst via een using statement.

Als ik classes maak die member variables ( die de class ook beheert ) hebben die IDisposable implementeren laat ik die class ook altijd IDisposable implementeren.

[ Voor 29% gewijzigd door Woy op 28-04-2009 09:56 ]

[Verwijderd]

Woy schreef op dinsdag 28 april 2009 @ 09:52:
Als ik classes maak die member variables ( die de class ook beheert ) hebben die IDisposable implementeren laat ik die class ook altijd IDisposable implementeren.

Volgens mij is dat zelfs een guideline die Microsoft ergens op MSDN geeft voor het gebruik van het IDisposable pattern.

edit: @woy: heb de guideline maar ff opgezocht: Implementing Finalize and Dispose to Clean Up Unmanaged Resources. Dat code project artikel is ook een goede inderdaad.

[ Voor 23% gewijzigd door Verwijderd op 28-04-2009 11:33 ]

[Woy]

Verwijderd schreef op dinsdag 28 april 2009 @ 10:32:
[...]

Volgens mij is dat zelfs een guideline die Microsoft ergens op MSDN geeft voor het gebruik van het IDisposable pattern.

Ja er is een complete guideline over, ook over hoe je IDisposable zou moeten implementeren. Het volgende artikel vind ik wel aardig over dit onderwerp: http://www.codeproject.com/KB/dotnet/idisposable.aspx