Code Verbesserung

  • C#
  • .NET (FX) 4.5–4.8

Es gibt 14 Antworten in diesem Thema. Der letzte Beitrag () ist von MichaHo.

    Code Verbesserung

    Hallo,
    wollte hier auch mal einen Code abgeben und fragen, was zu verbessern wäre.
    Erstmal, es geht darum, Termine eines Besprechungsraums abzufragen und zu zeigen ob der Raum zum jetzigen Zeitpunkt frei oder gebucht ist.
    Der Code funktioniert bestens, aber eventuell kann man da noch was verbsssern bevor ich mir eine dll erstelle und an die grafische Oberfläche gehe...
    Hier die Klasse EWSConnect:
    Spoiler anzeigen

    C#-Quellcode

    1. using System;
    2. using System.Collections.Generic;
    3. using System.Linq;
    4. using System.Net;
    5. using Microsoft.Exchange.WebServices.Data;
    7. namespace EWS
    8. {
    9. class EWSConnect
    10. {
    11. //Properties
    12. public string mailBox { get; set; }
    13. public string user { get; set; }
    14. public string pass { get; set; }
    15. public string domain { get; set; }
    16. public DateTime startDate { get; set; }
    17. public DateTime endDate { get; set; }
    18. public int maxItems { get; set; }
    19. public bool IsRoomFree { get; private set; }
    21. //Fields
    22. DateTime timeNow = DateTime.Now;
    24. //Constructor
    25. public EWSConnect(string mailBox, string user, string pass, string domain, DateTime startDate, DateTime endDate, int maxItems)
    26. {
    27. this.mailBox = mailBox;
    28. this.user = user;
    29. this.pass = pass;
    30. this.domain = domain;
    31. this.startDate = startDate;
    32. this.endDate = endDate;
    33. this.maxItems = maxItems;
    34. }
    35. //Function
    36. public FindItemsResults<Appointment> GetAppointments()
    37. {
    38. NetworkCredential netCred = new NetworkCredential(this.user, this.pass, this.domain);
    39. ExchangeVersion exVers = ExchangeVersion.Exchange2013;
    40. ExchangeService exService = new ExchangeService(exVers);
    41. exService.Credentials = netCred;
    42. exService.AutodiscoverUrl(this.mailBox);
    43. CalendarFolder calFolder = CalendarFolder.Bind(exService, WellKnownFolderName.Calendar);
    44. CalendarView calView = new CalendarView(this.startDate, this.endDate, this.maxItems);
    45. calView.PropertySet = new PropertySet(BasePropertySet.FirstClassProperties);
    46. FindItemsResults<Appointment> appointments = calFolder.FindAppointments(calView);
    47. IEnumerable<Appointment> result = from a in appointments where timeNow >= a.Start && timeNow <= a.End select a;
    48. if (result.Any()) { IsRoomFree = false; } else { IsRoomFree = true; }
    49. return appointments;
    50. }
    52. //Method
    53. public string CheckAllocation(string room)
    54. {
    55. IEnumerable<Appointment> busyResult = from a in GetAppointments() where timeNow >= a.Start && timeNow <= a.End select a;
    56. IEnumerable<Appointment> freeResult = (from a in GetAppointments() where a.Start >= timeNow select a).Take(1);
    57. string message = "";
    58. if (!IsRoomFree)
    59. {
    60. foreach (Appointment a in busyResult)
    61. {
    62. message = $"Der Raum {room} ist belegt, bitte anderen Raum buchen \nGebucht: {a.Start:d}, {a.Start:t} - {a.End:t} Uhr";
    63. }
    64. }
    65. else
    66. {
    67. foreach (Appointment a in freeResult)
    68. {
    69. message = $"Der Raum {room} ist frei und kann gebucht werden. \nNächste Buchung: {a.Start:d}, {a.Start:t} - {a.End:t} Uhr";
    70. }
    71. }
    72. return message;
    73. }
    74. }
    75. }


    der Aufruf erfolgt dann Beispielhaft so:
    Spoiler anzeigen

    C#-Quellcode

    1. using System;
    2. namespace EWS
    3. {
    4. class Program
    5. {
    6. static void Main(string[] args)
    7. {
    8. DateTime dateFrom = DateTime.Now.AddDays(-1);
    9. DateTime dateTo = dateFrom.AddDays(14);
    10. string room = "";
    11. EWSConnect meeting_application = new EWSConnect("MailBox", "UserName", "Password", "Domain",dateFrom, dateTo, 10);
    12. room = "Application Office";
    13. Console.WriteLine(meeting_application.CheckAllocation(room));
    15. EWSConnect meeting_bioCel = new EWSConnect("MailBox", "UserName", "Password", "Domain",dateFrom, dateTo, 10);
    16. room = "BioCel";
    17. Console.WriteLine(meeting_bioCel.CheckAllocation(room));
    19. EWSConnect meeting_aquadyn = new EWSConnect("MailBox", "UserName", "Password", "Domain",dateFrom, dateTo, 10);
    20. room = "Aquadyn";
    21. Console.WriteLine(meeting_aquadyn.CheckAllocation(room));
    22. }
    23. }
    24. }


    die Ausgabe auf der Console sieht so aus:

    Da ich mich in Zukunft mehr mit C# beschäftigen will/muss, wären Eure Kommentare sehr hilfreich...
    Danke Euch
    Gruß Micha
    "Hier könnte Ihre Werbung stehen..."

    MichaHo schrieb:

    C#-Quellcode

    1. if (result.Any()) { IsRoomFree = false; } else { IsRoomFree = true; }

    Das würde ich ehrlich gesagt mit "offenen" Klammern schreiben, also in 8 Zeilen, ist jedoch weder Styleguide noch sonst irgendwas, nur meine eigene Meinung.
    Ansonsten kann man (wenn du auf möglichst Kompaktn Code aus bist) in sämtlichen Kontrollstrukturen (if, for, foreach, etc) einen Einzeiler ohne die Klammern schreiben:

    MichaHo schrieb:

    C#-Quellcode

    1. if (!IsRoomFree)
    2. foreach (Appointment a in busyResult)
    3. message = $"Der Raum {room} ist belegt, bitte anderen Raum buchen \nGebucht: {a.Start:d}, {a.Start:t} - {a.End:t} Uhr";
    4. else
    5. foreach (Appointment a in freeResult)
    6. message = $"Der Raum {room} ist frei und kann gebucht werden. \nNächste Buchung: {a.Start:d}, {a.Start:t} - {a.End:t} Uhr";
    7. return message;

    bzw.

    MichaHo schrieb:

    C#-Quellcode

    1. if (result.Any()) IsRoomFree = false; else IsRoomFree = true;


    Edit: Was würde ich dafür geben mit C#7 arbeiten zu können ;(

    Dieser Beitrag wurde bereits 1 mal editiert, zuletzt von „EaranMaleasi“ ()

    Hi @EaranMaleasi. Danke Dir. Halte meinen Code eigentlich immer gerne Kompakt, die Frage ist, ist das sinnvoll :) beim ersten oben würde ich persönlich sagen ja, da ist ein 1 Zeiler gut, bei der foreach Geschichte ist es glaub sinnvoller über mehr zeilen zu gehen. Wobei man sich wohl für eins entscheiden sollte, oder?

    Noch ne andere Frage: Könnte man dies auch auf mehrere Klassen aufteilen? Ist das sinnvoll? ich denke da werden noch mehr Methoden dabei kommen, wobei eben nur Methoden, also könnte man doch hingehen und ne Klasse Room machen, ne Klasse Appointment und dann ne Klasse Result, Result enthält dann alle Methoden, Appointment eben nur die eine Funktion um die Termine abzuholen und Room enthält nur den Konstruktor um einenn neuen Raum zu definieren...

    Fragen über Fragen...
    "Hier könnte Ihre Werbung stehen..."

    MichaHo schrieb:

    C#-Quellcode

    1. if (result.Any()) { IsRoomFree = false; } else { IsRoomFree = true; }
    What :?:

    C#-Quellcode

    1. IsRoomFree = !result.Any();
    Falls Du diesen Code kopierst, achte auf die C&P-Bremse.
    Jede einzelne Zeile Deines Programms, die Du nicht explizit getestet hast, ist falsch :!:
    Ein guter .NET-Snippetkonverter (der ist verfügbar).
    Programmierfragen über PN / Konversation werden ignoriert!

    Auch reine Geschmackssache, aber ich persönlich würde bei Deklarationen bei denen Typ direkt ersichtlich ist, z.B. hier

    C#-Quellcode

    1. NetworkCredential netCred = new NetworkCredential(this.user, this.pass, this.domain);
    2. ExchangeVersion exVers = ExchangeVersion.Exchange2013;
    3. ExchangeService exService = new ExchangeService(exVers);

    var benutzen.

    Grüße
    Vainamo

    Wieder reine Geschmackssache, aber ich persönlich finde diese SQL abfragen im Code auch nicht schön. Die gibt es auch nur sehr selten, zumindest keine ich keine Leute die das so machen. Das kann man auch nur mit Linq und Lambda abbilden. Z.B.

    C#-Quellcode

    1. IEnumerable<Appointment> busyResult = from a in GetAppointments() where timeNow >= a.Start && timeNow <= a.End select a;

    müsste doch auch so gehen oder?

    C#-Quellcode

    1. IEnumerable<Appointment> busyResult = GetAppointments().Where(a => timeNow >= a.Start && timeNow <= a.End);

    @Vainamo meinst Du dann so?

    C#-Quellcode

    1. var netCred = new NetworkCredential(this.user, this.pass, this.domain);
    2. var exVers = ExchangeVersion.Exchange2013;
    3. var exService = new ExchangeService(exVers);


    Kannst Du bisschen mehr erklären warum/wann man var benutzen sollte?

    @Bluespide: genau DAS hatte ich krampfhaft versucht aber zum verr.... nicht hinbekommen.... Danke Dir, das sieht um einiges Besser aus und ist für mich vor allem besser lesbar
    "Hier könnte Ihre Werbung stehen..."

    var soll quasi zum schnellen Programmieren beitragen, indem du eben nicht mehr ellenlange Typen austippen musst, sondern eben nur diese drei Buchstaben.
    In Fällen wo du über new neue Objekte von Klassen erstellst, kann man es getrost einsetzen, die Klasse steht ja nach wie vor da.
    Kritisch wirds wenn du ne Factory oder ähnliches hast, und der Typ eben nicht klar ersichtlich ist, da du ne .Create() Funktion hast.
    Anstatt sofort zu sehen welche Klasse dort initialisiert wird, musst du auf IntelliSense warten.

    Meine Meinung:
    Da gerade IntelliSense immer "schlauer" wird was die auswahl der Klasse angeht wenn man gerade Tippt, fällt für mich das Argument der Geschwindigkeit eher Flach, da man oftmals nicht mehr als 4 Buchstaben tippen braucht bis zum Tab. Einziges Argument wäre noch übersichtlichkeit von Code, was ich auch irgendwie verstehe denn UberAwesomeFunctionalityProvider ließt sich halt doch schwerer als var. Dennoch ziehe ich es vor, immer den Typ direkt zu sehen.

    @EaranMaleasi OK, klingt plausibel. Wobei ich solche Kosntrukte

    C#-Quellcode

    1. NetworkCredential networkCredentials = new NetworkCredential(this.user, this.pass, this.domain)
    schon blöd finde :)
    Ich denke ich werd es dann so machen, das ich bei eindeutigen dingen (wie obiges Beispiel) die var deklaration nutze und bei allen anderen eben den richtigen Typ. Mal schauen.

    Bin da noch auf ein anderes Problem bei meinem obigen Code gestoßen.
    Wenn der Besprechungsraum nun von heute 14:00 Uhr bis morgen 16:00 Uhr gebucht wurde, steht in der Gui Gebucht: 15.03.17 14:00 Uhr - 16:00 Uhr somit denken die Leuts dann, das der Raum morghen Früh um 08:00 Uhr wieder frei ist. eigentlich müsst in solch einem Fall da stehen Gebucht: 15.03.17 14:00 Uhr - 16.03.17 16:00 Uhr
    Ich bekomme es aber schlichtweg nicht hin. Dachte ich müsste doch eigentlich nur prüfen ob Start und Ende den gleichen Tag haben... geht aber nicht... hier mein kläglicher Versuch:

    C#-Quellcode

    1. foreach (Appointment a in busyResult)
    2. {
    3. if(a.End.Day != a.Start.Day)
    4. message = $"{room}\nDer Raum ist belegt, bitte anderen Raum buchen. \nGebucht: {a.Start} - {a.End}";
    5. message = $"{room}\nDer Raum ist belegt, bitte anderen Raum buchen. \nGebucht: {a.Start:d}, {a.Start:t} - {a.End:t} Uhr";
    6. }

    Also wenn der Meeting Ende Tag nicht dem Meeting Start Tag entspricht, dann schreibe Datum plus Uhrzeit von Start und Ende, ansonsten ist es der gleiche Tag, da reicht dann Datum + Uhrzeit von bis...
    der Code springt IMMER in den 2. message Baum...
    "Hier könnte Ihre Werbung stehen..."

    @EaranMaleasi japp, jetzt hab ich es auch geschnallt :)
    ich überschreibe ja quasi die Variable message nach dem if wieder....
    weil ich da kein Return drinn habe, hab ich das komplett übersehen.

    Nun funktioniert es.... Dankeschön
    "Hier könnte Ihre Werbung stehen..."

    @MichaHo Ja, so war das gedacht. Hier z.B. eine Erklärung von ReSharper: jetbrains.com/help/resharper/2…rOrType_BuiltInTypes.html
    Wie gesagt, var ist reine geschmackssache aber vor allem in Using Konstrukten oder bei ewig langen Deklarationen bei denen Instanz und Klasse evtl sogar gleich heißen, schafft es einfach Übersichtlichkeit. Zudem kann man auch mal relativ schnell den Typen einer Variable ändern, weil man z.B. doch SHA512 haben möchte.

    C#-Quellcode

    1. SHA256CryptoServiceProvider CryptoServiceProvider = new SHA256CryptoServiceProvider();
    2. // vs
    3. var CryptoServiceProvider = new SHA256CryptoServiceProvider();


    Grüße
    Vainamo

    Hi @Vainamo und @ErfinderDesRades... Danke für Eure Erklärungen. den Link schau ich mir später noch an...
    Ich werds so machen wie oben schon erwähnt, wenn der Typ eindeutig ist nehm ich var, ansonsten eben den Typ....
    Danke
    "Hier könnte Ihre Werbung stehen..."

Facebook
Telefonspende