Uhu, dawno nie pisałem. Już już, nie płaczcie.
Dlaczego warto rozmawiać, pytać, poprawiać i pokazać komuś swój kod? Najtrudniej jest znaleźć swoje błędy, trudno jest spojrzeć na swój kod i zapytać się “czemu zrobiłem to tak – a nie w inny sposób?”. Skoro wszystko działa tak jak założyłem na początku to znaczy, że jest dobrze i refaktoryzacja jest nie potrzebna. Jeśli się uruchamia, działa i nie wywala to po co mi napisać testy. I w końcu skoro się tyle namęczyłem na tym wszystkich, użyłem całej swojej wiedzy, internetu i haków gdzieś znalezionych to czemu mam to komuś pokazać? To moja wiedza tajemna, nie chce się tym dzielić, nie chce żeby ktoś zobaczył mój workaround na jakiś problem.
Testy, idea TDD pewnie każdy słyszał i mówił że super i fajnie, że trzeba i co? I życie, nie chce się pisać. Jak też nie pisałem testów, bo to projekt gdzie się będę uczyć nowego, że więcej czasu spędzę nad testami niż napisaniem właściwego kodu. Teraz siedzę sobie na testami i wychodzą różne kwiatki, które napisałem na szybko, na chwilę, żeby się skompilowało. Można powiedzieć że robię review samemu sobie. Taki przykładzik:
.csharpcode, .csharpcode pre
{
font-size: small;
color: black;
font-family: Consolas, “Courier New”, Courier, Monospace;
background-color: #ffffff;
max-height: 300px;
overflow: auto;
/*white-space: pre;*/
}
.csharpcode pre { margin: 0em; }
.csharpcode .rem { color: #008000; }
.csharpcode .kwrd { color: #0000ff; }
.csharpcode .str { color: #a31515; }
.csharpcode .op { color: #0000c0; }
.csharpcode .preproc { color: #cc6633; }
.csharpcode .asp { background-color: #ffff00; }
.csharpcode .html { color: #800000; }
.csharpcode .attr { color: #ff0000; }
.csharpcode .alt
{
background-color: #f4f4f4;
width: 100%;
margin: 0em;
}
.csharpcode .lnum { color: #606060; }
1: namespace CompactCal.Model
2: {
3: using System;
4: using System.Collections.Generic;
5: using Microsoft.Phone.UserData;
6:
7: public class CalendarEntry
8: {
9: public CalendarEntry(Appointment appointment)
10: : this(
11: appointment.Attendees,
12: appointment.Details,
13: appointment.StartTime,
14: appointment.EndTime,
15: appointment.IsAllDayEvent,
16: appointment.Location,
17: appointment.Organizer,
18: appointment.Subject)
19: {
20: }
21:
22: public CalendarEntry()
23: {
24:
25: }
26:
27: public CalendarEntry(
28: IEnumerable<Attendee> attendees,
29: string details,
30: DateTime startTime,
31: DateTime endTime,
32: bool isAllDay,
33: string location,
34: Attendee organizer,
35: string subject)
36: {
37: this.Attendees = new List<CalendarPerson>();
38: foreach (var a in attendees)
39: {
40: this.Attendees.Add(new CalendarPerson(a));
41: }
42: this.Details = details;
43: this.StartTime = startTime;
44: this.EndTime = endTime;
45: this.IsAllDayEvent = isAllDay;
46: this.Location = location;
47: if (organizer != null)
48: {
49: this.Organizer = new CalendarPerson(organizer);
50: }
51: this.Subject = subject;
52:
53: this.Duration = this.EndTime.Subtract(this.StartTime);
54: }
55:
56: public CalendarEntry(
57: IEnumerable<CalendarPerson> attendees,
58: string details,
59: DateTime startTime,
60: DateTime endTime,
61: bool isAllDay,
62: string location,
63: CalendarPerson organizer,
64: string subject)
65: {
66: this.Attendees = new List<CalendarPerson>();
67: this.Attendees.AddRange(attendees);
68: this.Details = details;
69: this.StartTime = startTime;
70: this.EndTime = endTime;
71: this.IsAllDayEvent = isAllDay;
72: this.Location = location;
73: this.Organizer = organizer;
74: this.Subject = subject;
75:
76: this.Duration = this.EndTime.Subtract(this.StartTime);
77: }
78:
79: public List<CalendarPerson> Attendees { get; private set; }
80:
81: public string Details { get; private set; }
82:
83: public DateTime StartTime { get; private set; }
84:
85: public DateTime EndTime { get; private set; }
86:
87: public bool IsAllDayEvent { get; private set; }
88:
89: public string Location { get; private set; }
90:
91: public CalendarPerson Organizer { get; private set; }
92:
93: public string Subject { get; private set; }
94:
95: public TimeSpan Duration { get; private set; }
96: }
97: }
I co widać, syfek. Kod dla konstruktorów się powtarzają, w ogóle jest ich za dużo. Same dziwy ;). Czemu tak? Bo mi się śpieszyło żeby rozwiązać jakiś problem i tak zostawiłem, a skoro działało to nie wracałem do tematu. Dopiero gdy zabrałem się za testowanie poprawiłem kod na taki który mnie bardziej zadowala:
1: using System;
2: using System.Collections.Generic;
3: using System.Linq;
4: using Microsoft.Phone.UserData;
5:
6: namespace Model
7: {
8: public class CalendarEntry
9: {
10: public CalendarEntry(Appointment appointment) : this(
11: appointment.Attendees.ConvertToCalendarPersons(),
12: appointment.Details,
13: appointment.StartTime,
14: appointment.EndTime,
15: appointment.IsAllDayEvent,
16: appointment.Location,
17: appointment.Organizer.ConvertToCalendarPerson(),
18: appointment.Subject)
19: {
20: }
21:
22: public CalendarEntry(
23: IEnumerable<CalendarPerson> attendees,
24: string details,
25: DateTime startTime,
26: DateTime endTime,
27: bool isAllDay,
28: string location,
29: CalendarPerson organizer,
30: string subject)
31: {
32: Attendees = new List<CalendarPerson>();
33: Attendees.AddRange(attendees);
34: Details = details;
35: StartTime = startTime;
36: EndTime = endTime;
37: IsAllDayEvent = isAllDay;
38: Location = location;
39: Organizer = organizer;
40: Subject = subject;
41:
42: Duration = EndTime.Subtract(StartTime);
43: }
44:
45: public List<CalendarPerson> Attendees { get; private set; }
46:
47: public string Details { get; private set; }
48:
49: public TimeSpan Duration { get; private set; }
50:
51: public DateTime EndTime { get; private set; }
52:
53: public bool IsAllDayEvent { get; private set; }
54:
55: public string Location { get; private set; }
56:
57: public CalendarPerson Organizer { get; private set; }
58:
59: public DateTime StartTime { get; private set; }
60:
61: public string Subject { get; private set; }
62: }
63: }
Co się zmieniło? Wywaliłem dwa konstruktory, jeden z nich okazał się w ogóle nie używany. A drugi był nadmiarowy. Kod jest krótszy i się nie powtarza. Prostszy do utrzymania i ewentualnych poprawek czy zmian. Wszystko dzięki review, napisaniu kilku linijek kodu testowego, oraz chwili przemyślenia. Myślę że gdyby ten kod zobaczył ktoś inny, wcześniej, zanim wybrałem opcję commit nie doszło bo takiej sytuacji, gdzie zmiany byłyby potrzebne. Najprawdopodobniej musiałbym wcześniej to poprawić zanim będę mógł wrzucić zmiany na serwer. Ale pracuje sam, gdzie nie ma mnie kto opieprzyć. Review kodu to naprawdę dobra, szybka i tania możliwość znalezienia prostych błędów. Jeśli tylko jest taka możliwość, aby ktoś przejrzał nasz kod, należy z tego skorzystać.
Opowiadałem kilku znajomym co zrobiłem, jak to zrobiłem, i że w ogóle takie fajne. Padło stwierdzenie że niezły hack, ale nie jest to do końca czyste zagranie. Chodzi o deklarację interfejsu, który będzie posiadać event. Ale od początku: napisałem własnego delegata, później event, F7, build i błąd: interfejs nie może definiować własnych typów. Pierwszy odruch google “how to declare event in interface” – pierwszy wynik MSDN, wszystko pięknie, CTRL+C/V – super, działa, walczę dalej.
1: namespace ImplementInterfaceEvents
2: {
3: public interface IDrawingObject
4: {
5: event EventHandler ShapeChanged;
6: }
7: public class MyEventArgs : EventArgs
8: {
9: // class members
10: }
11: public class Shape : IDrawingObject
12: {
13: public event EventHandler ShapeChanged;
14: public void ChangeShape()
15: {
16: // Do something here before the event…
17: OnShapeChanged(new MyEventArgs(/*arguments*/));
18: // or do something here after the event.
19: }
20: protected virtual void OnShapeChanged(MyEventArgs e)
21: {
22: if(ShapeChanged != null)
23: {
24: ShapeChanged(this, e);
25: }
26: }
27: }
28:
29: }
To kod z MSDN, potem takiego eventa w obsłudze trzeba rzutować na MyEventArgs jeśli chce się odczytać dodatkowe właściwości z argumentu. Nawet się nie zastanowiłem, że mogę coś zrobić inaczej, skoro MSDN tak pokazuje to powinno być dobrze. Właśnie do tego rozwiązania padł zarzut, że to niezły trik/hack, ale nie jest czysty. Że lepiej jest zdefiniować własny typ dla delegata i z niego korzystać w interfejsie. Jak – skoro wywala błąd (czasem jestem taki głupiutki)?!.
1: using System;
2:
3: namespace GeocodeService
4: {
5: public delegate void QueryDelegate(object sender, GeoLocation location);
6:
7: public interface IGeocode
8: {
9: void Query(string aLocation);
10: event QueryDelegate QueryCompleted;
11: }
12:
13: }
Ech 🙂 Teraz nie trzeba nic rzutować, wszystko jest proste i czyste. Pewnie gdybym nie pokazał tego swojego cuda, to został bym z przykładem z msdn i byłbym z niego zadowolony. Żyłbym sobie w błogiej nieświadomości.
Jeśli piszesz solo staraj się używać TDD, to pierwszy front kontroli twojej jakości kodu który tworzysz. Uważasz że TDD to strata czasu, napisz od czasu do czasu prosty test, sprawdź czy to napisałeś wtedy, nadal jest potrzebne teraz i czy spełnia wszystkie założenia.
Poproś kogoś aby zerknął na kawałek kodu, jeśli nie zrozumie tego co widzi to znaczy że może jest to przekombinowane, może da się uprościć. Przeczytaj czasem kod kolegi, zapytaj się czemu napisał to tak a nie inaczej, może Ty dowiesz się czegoś nowego, lub on zastanowi się nad tym co napisał i to poprawi. O kodzie, o tym jak jest napisany i pomyślany naprawdę rozmawiać.
Co o tym sądzicie? Mam rację? Gadam głupoty?
