Robert Haken [MVP ASP.NET/IIS, MCT] software architect, HAVIT, s.r.o.
[email protected], @RobertHaken
Perly code-review z praxe
Task: Přidej do sloupce Název info-baloon
Co je code-review? druhé vývojářské oči – komunikace dvou (nebo více) rovnocennýchstran
revize a návrhyna vylepšení – zvoleného způsobu implementace – srozumitelnosti – kvality implementace obecně
rychlá zpětná vazba nástroj zvyšování kvality produktu nástroj zlepšování týmu
Výsledek:
Výsledek:
Co není code-review? testování kontrola dodržování standardů kódování – StyleCop, FxCop(CodeAnalysis)
Formy code-review pre-commit review post-commit review příležitostné konzultace pair-programming
lock(new object()) { // thread-safe }
...všude píšou, že stačí lockna newobject private object myLock = new object();
lock(myLock) { ... }
Kategorie
Provizornosti
/// <summary> /// Vrací seznam všech záznamů z tabulky LoginLog. /// /// <param name="dateFrom">Datum od pro dotazy /// <param name="dateTo">Datum do pro dotazy ///
Vrací kolekci objektů typu "LoginLog" s načtenými daty. public static IEnumerable
GetListChartLoginLog( DateTime dateFrom, DateTime dateTo) { using (Context ctx = new Context()) { return ctx.LoginLogs .Where(x => x.Date >= dateFrom && x.Date <= dateTo && x.Login) .Select(x => x.Date).OrderBy(x => x).ToList(); } }
Pečlivá dokumentace. Copy-Paste?
public bool KontrolaParametru(out string message) { message = string.Empty; return true; }
// WTF? Jednotka dočasnosti = 1 FURT
SELECT Faktura.FakturaID FROM Faktura WHERE (1 = 1) AND (Deleted IS NULL) AND (SmerFaktury = @SmerFaktury) AND (Faktura.TypFakturyID IN (SELECT Value FROM @TypFakturyID))
...aby mi to hezky zarovnávalo
public static Task RunDelayed(int milliseconds, Action action) { return Task.Run(async delegate { await Task.Delay(1000); action(); }); }
Tzv. tvrdý default?
Proč code-review? Vývojář -autor – Dostávám zpětnou vazbu a učím se. – Vím, že to po mě někdo uvidí. Snažím se, abych se za to nestyděl.
Vývojář -reviewer – Inspiruji se prací jiných, učím se. – Stojí o můj názor, realizuji se. – Chci-li někomu něco vytýkat, sám to musím dělat správně.
Tým – Zkracuje se doba na odhalení některých problémů -snažšíkorekce. – Zrychluje se adaptace nových členů týmu. – Posiluje se týmové pojetí projektu.
Kategorie
Věřím si, netestuji
MySuperGrid.SortExpression = "Date.DayOfWeek";
DateTime.DayOfWeek Property Remarks The value of the constants in the DayOfWeek enumeration ranges from DayOfWeek.Sunday to DayOfWeek.Saturday. If cast to an integer, its value ranges from zero (which indicates DayOfWeek.Sunday) to six (which indicates DayOfWeek.Saturday).
Prvním dnem je neděle. :-(
private void BindData() { var procesy = Process.GetProcesses(); procesy.OrderBy(i => i.ProcessName);
MySuperGrid.DataSource = procesy; }
„...ty systémový věci jsou nějaký divný.“ private void BindData() { var procesy = Process.GetProcesses(); MySuperGrid.DataSource = procesy.OrderBy(i => i.StartTime); }
[Flags] public enum TreeNodeType { Normal, Linked, Locked } if (t.HasFlag(TreeNodeType.Normal)) { // WTF? always true! }
[Flags] vždy s hodnotami, jinak je Normal= 0, Linked= 1, Locked= 2, ... [Flags] public enum TreeNodeType { Normal = 1, Linked = 2, Locked = 4 // 8, 16, ... }
Kategorie
Proč to dělat jednoduše, když to jde složitě?
query.DateTo = DateTime.Today.Date;
...pro jistotu. // DateTime class public static DateTime Today { get { return Now.Date; } }
// Parsuje z formátu 19901231 public static DateTime? DateFromXmlString(string value) { if (string.IsNullOrEmpty(value)) return null; return DateTime.Parse( string.Format( "{2}.{1}.{0}", value.Substring(0, 4), value.Substring(4, 2), value.Substring(6, 2)), CultureInfo.CreateSpecificCulture("cs-CZ"), DateTimeStyles.None); } public static DateTime? DateFromXmlString(string value) { DateTime date; if (DateTime.TryParseExact(value, "yyyyMMdd", CultureInfo.CreateSpecificCulture("cs-CZ"), DateTimeStyles.None, out date)) { return date; } return null; }
C# 6.0 -Declaration expressions public static DateTime? DateFromXmlString(string value) { if (DateTime.TryParseExact(value, "yyyyMMdd", CultureInfo.CreateSpecificCulture("cs-CZ"), DateTimeStyles.None, out var date)) { return date; } return null; }
public int GetHashCode(DuplicitaOvereniKontrolniAkceStructure obj) { if (!obj.OvereniID.HasValue) return string.Empty.GetHashCode();
return obj.OvereniID.Value.GetHashCode(); }
public int GetHashCode(DuplicitaOvereniKontrolniAkceStructure obj) { return obj.OvereniID.GetHashCode(); }
var posledniRadekProjekt = Faktura.Items .Last(o => o.Poradi == Faktura.Items.Max(v => v.Poradi)) .Projekt;
Poslední z posledních? .Last(predicate) // odpovídá .Where(predicate).Last()
int fakturaID = result.Faktura.ID; Faktura faktura = (fakturaID == 0) ? result.Faktura : fakturaRepository.GetFakturaByID(fakturaID);
...takhle už to nevypadá tak machrovsky: Faktura faktura = result.Faktura;
DataSet dsResult = new DataSet(); SqlDataAdapter myDataAdapter = new SqlDataAdapter( "SELECT COUNT(*) FROM Uzivatel", "**ConnString**"); myDataAdapter.Fill(dsResult); string count = dsResult.Tables[0].Rows[0][0].ToString(); dsResult = null;
Není to náhodou ExecuteScalar? int count2 = (int)cmd.ExecuteScalar();
Task: Importuj jen druhy 2, 4 a 5 int[] nenacitaneDruhy = new int[] { 1, 3, 6, 7, 8, 9, 10, 11, 13, 14, 15, 16, 17, 18, 19, 50, 51, 52, 53, 54 }; if (!nenacitaneDruhy.Contains(druhZakazky)) { // importuj }
Pozitivní přístup!
Kategorie
Performance killers
private readonly Dictionary<string, string> localizedUrls; private string GetUrl(string lang) { if (localizedUrls.ContainsKey(lang)) return localizedUrls[lang]; return "/"; }
private string GetUrl(string lang) { if (localizedUrls.TryGetValue(lang, out string url)) { return url; } return "/"; }
Proč hledat dvakrát?
private readonly Localization[] items; public string GetLocalization(string key, CultureInfo culture) { return items. Where(x => x.Equals(new Localization(key, culture))). Select(x => x.Text). FirstOrDefault(); } public bool Equals(Localization other) { if (object.ReferenceEquals(other, null)) return false; return (Key.Equals(other.Key) && LCID.Equals(other.LCID)); }
...kterak potěšit GarbageCollector stovky řetězců per Page tisíce Localizations v items >200 000 volání constructoru per Page
private void Page_Load() { // graf s počty přihlášení po dnech za dané období IEnumerable data = LoginLogService.GetListChartLoginLog( fromPicker.SelectedDate, toPicker.SelectedDate); DateTime dt = fromPicker.SelectedValue; List list = new List(); while (dt <= DateTime.Today) { list.Add(new LoginStatistic { Date = dt, Count = data.Where(x => x.Date == dt).Count() }); dt = dt.AddDays(1); } MyChart.DataSource = list; MyChart.DataBind(); }
Výkonová optimalizace! Stačí, když to spočítám do dneška, v budoucnu žádná data přihlášení nejsou.
Kategorie
Coding culture
// Kontrola vyplnění RČ a IČ dle právní formy if (akce.DotacePrijemcePravniFormaTyp == 1) chybaAkce = "Pro typ '1' musí být zadáno RČ a nesmí být vyplněno IČ!"; if (akce.DotacePrijemcePravniFormaTyp == 2) chybaAkce = "Pro typ '2' musí být alespoň jedna z položek RČ a IČ vyplněna!"; if (akce.DotacePrijemcePravniFormaTyp == 3) chybaAkce = "Pro typ '3' musí být zadáno IČ a nesmí být vyplněno RČ!"; public enum PravniForma { Nepodnikatel = 1, Zivnostnik = 2, PravnickaOsoba = 3 }
MagicNumbersNEEE!
switch (akce.DotacePrijemcePravniFormaTyp) { case PravniForma.Soukromnik: chybaAkce = "..."; break; case PravniForma.Zivnostnik: chybaAkce = "..."; break; case PravniForma.PravnickaOsoba: chybaAkce = "..."; break; }
#region SetButtonFakturovatVisibility public void SetButtonFakturovatVisibility(bool value) { if (value) { FakturovatVybraneBT.Visible = true; } else { FakturovatVybraneBT.Visible = false; } } #endregion
Placen za počet řádek?
public DateTime? DateFrom { get { return this.dateFrom; } set { DateTime min = SqlDateTime.MinValue.Value; DateTime max = SqlDateTime.MaxValue.Value; if ((value.HasValue) && ((value.Value <= min) || (value.Value >= max))) this.dateFrom = null; else this.dateFrom = value; } }
...že by nová technika validace? DateFrom = input.Value; IsValid = (DateFrom != null);
DateFrom = input.Value ?? DateTime.Today; XyLabel.Text = DateFrom.Value.ToString("g");//Ex: Nullable object must have...
row.Cells["zbyvaCol"].Value = decimal.Parse(row.Cells["limitCol"].Value.ToString().Replace(".", ",")) - decimal.Parse(row.Cells["cerpanoCol"].Value.ToString().Replace(".", ","));
...asi ani nechci vědět, co tím autor myslel a jak se to chová v různých Culture.
public static Exception SendAlert(string subject, string body) { Exception exceptionOut = null; try { MailMessage message = new MailMessage(); message.From = new MailAddress("[email protected]"]); message.To.Add(new MailAddress("[email protected]")); message.Subject = subject; message.Body = body; (new SmtpClient(ConfigurationManager.AppSettings["SMTP"])).Send(message); } catch (Exception exception) { exceptionOut = exception; } return exceptionOut; } var ex = SendAlert(subject, body); if (ex != null) // ?? catch (ex) { Log(ex); }
Pokud výjimku nezpracováváte, tak ji nezachycujte!
private void GetListForMonth(SqlConnection sqlConnection, int month, int year, Dictionary<string, bool> list) { list.Clear(); // ... while (sqlDataReader.Read()) { list.Add(sqlDataReader[0].ToString(), true); } // ... }
var list = GetListForMonth(conn, 10, 2014,
var list = new List<string>(); GetListForMonth(conn, 10, 2014, list);
Kategorie
Klasický bugy
protected Dictionary<string, string> _queryParams = new Dictionary<string, string>(); public void SetQueryParams(Dictionary<string, string> queryParams) { _queryParams.Clear(); _queryParams = queryParams; }
var myQueryParams = new Dictionary<string, string>() { { "ICO", "25612697" }, { "NAZEV", "XY" } }; resolver.SetQueryParams(myQueryParams); // _queryParams = myQueryParams var result1 = resolver.Execute(); myQueryParams.Add("DIC", "CZ25612697"); resolver.SetQueryParams(myQueryParams); // myQueryParams.Clear() var result2 = resolver.Execute();
„...hledání s DIČ jimnefunguje!“
Bug: Streamvrácený metodou GetMemoryStreamnení disposován Oprava: ...doplněn using
http://knowledge-base.havit.cz
Q&A
