Ich unterstütze derzeit eine Anwendung bei der Arbeit, die ursprünglich von einem vierköpfigen Team geschrieben wurde, jetzt aber auf mich reduziert ist. Wir haben vor kurzem einen Vertragspartner, um einige Leistungsprobleme zu sehen, während ich mit anderen Dingen beschäftigt bin.
Während der Auftragnehmer mit der Aufführung anscheinend gute Arbeit geleistet hat, haben sie auch große Mengen des Codes durchlaufen, die den bestehenden Stil mit ihren persönlichen Vorlieben ersetzen.
Leider haben wir kein Kodierungsstandards-Dokument, nur eine allgemeine Regel, um c # allgemeine Regeln einzuhalten.
Als Beispiel für das, was sie getan haben, enthält es:
Wir betreiben auch eine TDD-Richtlinie, aber die Testabdeckung, besonders bei den leistungsspezifischen Teilen, ist sehr gering und lässt nur sehr wenig Dokumentation darüber übrig, was sie geändert haben und macht es noch schwieriger, da ihre Checkin-Kommentare nicht besonders hilfreich sind Die tatsächlichen funktionalen Veränderungen gehen zwischen den einzelnen "Tweaks" verloren.
Wie spreche ich mit dem Auftragnehmer darüber? Offensichtlich gibt es keinen großen Anreiz, sie zu ändern, da sie nicht die Verantwortung haben, das Projekt zu unterstützen, und sie scheinen für Veränderungen nicht sonderlich aufgeschlossen zu sein.
Oder sollte ich es nur für die kurze Dauer des Vertrags dulden und dann alles auf die Codeformatierung zurückstellen, die wir vorher benutzt haben?
Made Community-Wiki, weil es hier wahrscheinlich keine richtige Antwort gibt.
Überall mit einer if-Anweisung und einer einzelnen Zeile haben sie geschweifte Klammern hinzugefügt
Dieser und der einzige kann nützlich sein.
- fast alle Verwendungen des Schlüsselworts 'var' werden gelöscht
- Entfernen der meisten lambdas und Ersetzen durch ausführlicheren Code
- Ändern der Methodensignaturen, so dass sich alle Parameter in einer separaten Zeile und nicht in einer Zeile befinden
Diese machen wenig Sinn zu ändern.
Sagen Sie ihm, dass er nicht berechtigt ist, Code neu zu gestalten. Sie werden nicht für die Zeit bezahlen, die für diese Aktivitäten verschwendet wird, und sie müssen ihre eigene Zeit nutzen, um Dinge zurück zu legen. Das sollte eine Erfrischung bieten.
Diese Dinge sollten im Voraus besprochen werden. Sie sollten klar angeben, welche Aktivitäten zulässig sind und welche nicht. Vor nicht allzu langer Zeit gab es hier noch eine ähnliche Frage zu einem Auftragnehmer, der seine Initialen über den gesamten Code einschließlich der Datenbank-Entitäten stellen würde. Es war eine perverse Art von Eigenwerbung, für die es im Code von jemand anderem keinen Platz gibt.
P.S. Es kann auch die Möglichkeit bestehen, dass Ihr Auftragnehmer durch all diese Dinge künstlich zusätzliche Arbeitslast schafft, um Ihnen mehr Stunden in Rechnung zu stellen.
Ich bin ein Bauunternehmer (manchmal) und wenn ich das täte, würde ich erwarten, dass die Tür mit großer Geschwindigkeit und ohne Bezahlung gezeigt wird. Ernsthaft, diese Person wird von Ihnen angestellt und sollte genau das tun, was ihm gesagt wurde, nicht mehr und nicht weniger. Und sorgen Sie sich nicht darum, "nett" zu sein - Unternehmer erwarten das nicht von Permies.
Wie spreche ich mit dem Auftragnehmer darüber?
Höflich: Erklären Sie, warum Sie Änderungen am Quellcode minimieren möchten.
Alternativ können Sie vor dem Einchecken eine Code-Prüfung der Änderungen durchführen: und das Einchecken von Änderungen, die Sie nicht verstehen / nicht wollen / nicht getestet haben, nicht zulassen.
Implementiere FxCop - das sollte deine erste Verteidigungslinie sein. Wenn Sie Quellcodeverwaltung verwenden (wenn Sie dann nicht einen ASAP implementieren), stellen Sie sicher, dass Sie dev labelling verwenden (bauen Sie nur auf Dateien auf, die für den Build benannt wurden) und geben Sie ihm keine Rechte zum Verschieben von Labels auf dem Dateien. Auf diese Weise können Sie seine Änderungen überprüfen und sich weigern, seinen Code so lange zu kennzeichnen, bis er Ihren Standards entspricht. Was auch immer er kodiert, wird es erst in QA schaffen, wenn du das Entwickler-Label auf die betreffende Version verschiebst, so dass er dir dort sehr ausgeliefert ist. Beachten Sie, dass einige Shops kein einzelnes Label für ihre Sandbox-Builds verwenden. Sie möchten sogar neue Labels selbst in der Sandbox anwenden, daher könnten Sie auch dazu neigen.
Das Problem ist jetzt passiert, und wie der andere sagte, es ist eine ungerechtfertigte Verschwendung Ihres Geldes und es ist völlig unhöflich (so korrekt wie die geschweiften Klammern Sache sein kann).
Um zukünftige Probleme zu vermeiden und möglicherweise hilfreich bei der Lösung dieses Problems, rate ich Ihnen, ein stylycop
Ich denke, wir alle kennen die Versuchung, Codierungen zu sehen, von denen wir denken, dass sie "nicht so sind, wie ich es tun würde". Aber wir widerstehen ihm.
Ich würde mich mit deinem Chef darüber unterhalten, um sie zu verstehen. Aber das erste, was einem in den Sinn kommt, ist, dass, wenn Sie den Auftragnehmer nicht ausdrücklich gebeten haben, die Arbeit zu tun, er nicht das getan hat, wofür er eingestellt wurde, ungeachtet dessen, was er seiner Meinung nach hinzugefügt hat. Also muss darüber diskutiert werden.
Das nächste, was mir in den Sinn kam, ist, dass Leute, die Massenänderungen vornehmen, ohne mit den Besitzern des Codes darüber zu sprechen, schlechte Nachrichten sind, egal wie gut sie auch sein mögen. Sie werden Leute verärgern oder schlimmere Fehler und unvorhergesehenes Verhalten einführen, das Sie aufräumen müssen. Er muss darauf eingestellt sein, dass es nicht akzeptabel ist, so etwas zu tun, ohne eine Erlaubnis für den Code anderer zu haben.
Wenn ich Dinge sehe, die ich in anderen nicht mag, die ernst genug sind, um Aufmerksamkeit zu erregen, frage ich zuerst mit den Besitzern des Codes nach. Auch wenn es offensichtliche Bugs gibt s ihr Code und ihre Entscheidung, es aufzuräumen, nicht meins.
Wie andere bereits gesagt haben, dienen diese Änderungen lediglich der Codierung. Wenn er da ist, um die Leistung zu verbessern, verschwendet er Zeit mit diesen Änderungen. Wenn er nicht sagen kann, wie diese Änderungen die Leistung verbessern, dann läuft seine OCD gerade die Rechnung hoch.
Ich würde sagen: "Ich schätze Ihre Änderungen am Codierungsstil, aber wir können uns auf nicht stilbedingte Änderungen in Bereichen des Codes konzentrieren, die die Verlangsamung verursachen."
Wenn ein Unternehmer den Code ohne Genehmigung völlig neu formatiert hätte, würde ich ihm nur eine einzige Änderung geben, um die Dinge wieder so zu gestalten, wie sie waren - und zu seiner Zeit.
Betrachte zusätzlich zu den gültigen Punkten, die andere machen, den Albtraum der Versionskontrolle, der dies verursacht. Statt der sauberen Fortsetzung einiger Zeilen, die hier hinzugefügt wurden, wurden hier ein paar Zeilen geändert. Sie haben nun diesen "Riss" in Ihrer Quellcodeverwaltungsdatenbank, sodass Vergleiche zwischen Versionen vor und nach den "Verbesserungen" dieses Auftragnehmers bedeutungslos werden.
Lassen Sie den Auftragnehmer alle Änderungen rückgängig machen. Heute. Und zu seiner Zeit.
Das ist ziemlich häufig meine Erfahrung, dass Leute nicht widerstehen können, "Verbesserungen" zu machen und plötzlich stellt man fest, dass man für Dinge in Rechnung gestellt wird, die man nicht wollte. Manchmal bin ich sicher, dass es absichtlich getan wird, um mehr bezahlte Arbeit zu bekommen, aber meistens denke ich, dass es ein Entwickler ist, der die Seite verfolgt und nicht damit fertig wird, "falschen" Code zu hinterlassen. Es könnte ein bisschen Kampf erfordern, aber im Grunde muss man immer wieder wiederholen: "Ändere nichts, an dem du nicht arbeiten sollst". Abhängig von seiner Persönlichkeit, müssen Sie nur einmal nett fragen, oder jemand höher bringen, um ihn zu zwingen.
Erstens, wie andere gesagt haben. Du bezahlst die Rechnung. Er ist kein Angestellter. Seine Aufgabe ist es, das zu tun, was Sie von ihm verlangen, und nur das, was Sie von ihm verlangen, sonst können Sie ihm die Tür zeigen. Erinnere dich immer daran. Sie fahren das Boot, nicht er. Sie können versuchen, ihn nicht zu bezahlen, aber das wird schwer zu tun sein, wenn Sie einen legalen Vertrag haben und es nichts daran gibt, Code wie er ist zu verlassen. Aber Sie können ihn jederzeit einfach gehen lassen.
Zweitens, wenn du ihn nicht dazu bringen kannst, aufzuhören und zurückzugehen, und du ihn nicht loswerden kannst, kannst du ihm sagen, dass er, wenn er Stiländerungen vornehmen möchte, alle Stiländerungen in einem Scheck machen sollte -in mit absolut KEINE Codeänderungen. Auf diese Weise können Sie von einem Basissatz von Code aus vorgehen, der zur Anzeige von Codeänderungen freigegeben werden kann.
Drittens, lassen Sie ihn die Rechtfertigung für die Änderungen erklären, die er vorgenommen hat. Das Entfernen von var hat keinen Leistungsvorteil.
Viertens, und das mag sehr viel Spaß machen, aber du benutzt immer ReSharper, um den Code nach dem Fakt wieder auf deinen akzeptierten Stil zu setzen. Es ist mehr Arbeit und du hast immer noch Diffs geborcht, aber naja. Die Lambdas sind härter, und das ist die, über die du wirklich in seinem Fall reden solltest.
Fünftens, um Ihren Punkt nach Hause zu bringen, zwingen Sie ihn, jede Änderung, die er vorgenommen hat, zurückzunehmen und nur die Codeänderungen neu zu implementieren, und nicht die Stiländerungen. Das sollte seine Augen öffnen für das Chaos, das er geschaffen hat, wenn er es selbst nicht herausfinden kann.
Schließlich können Sie nur die Kugel in den sauren Apfel beißen und ihn belohnen. Ja, es ist scheiße, aber da du den Fehler gemacht hast, ihn nicht zu überwachen, nicht vorzuschreiben, was du wolltest und was er nicht tun darf ... Du wirst den ultimativen Preis bezahlen. Sie können ihn entweder dafür bezahlen, jemanden dafür bezahlen, Sie dafür bezahlen, oder damit leben (und den Preis für die geborkenen Diffs bezahlen). Wie auch immer Sie schneiden, es kostet Sie Geld.
Nun, riecht wie ein lösungsweiter Code, der mir reformatiert, der durch Einstellungen in einem Tool wie Resharper automatisiert / erzwungen werden könnte . Ich würde es sehr unhöflich finden und würde ihn bitten, den Knopf "Alles Code nach meinem persönlichen Geschmack ändern" nicht zu drücken.
Um zu vermeiden, dass die Situation an erster Stelle auftritt, führen Sie eine Code-Überprüfung ein, insbesondere für neue Entwickler, die sich Ihren Standards möglicherweise nicht anschließen.
Ich bin ein großer Fan von git, Feature-Branches und einem Service, der Pull-Requests unterstützt (github oder bitbucket). TFS ist dem Job nicht wirklich gewachsen, aber glücklicherweise unterstützt Visual Studio git jetzt. Die Code-Überprüfung vor dem Zusammenführen zum Master stellt sicher, dass sie nicht vergessen wird. Wenn Sie paranoid sind, müssen Sie den Auftragnehmern nicht einmal Schreibzugriff auf Ihr Primär-Repository geben.
Alternative Sichtweise:
Sie geben zwei Erklärungen ab: "Während der Auftragnehmer mit der Aufführung gute Arbeit geleistet zu haben scheint" und "haben sie auch große Mengen des Codes durchlaufen und den vorbestehenden Stil durch ihre persönlichen Vorlieben ersetzt."
Dies wirft viele Fragen auf wie: Wann immer Sie einen Auftragnehmer für eine kurze Zeit "hereinlegen" können und Leistungsverbesserungen erhalten. Dies deutet darauf hin, dass es in der Anwendung von Anfang an sehr große Fehler gegeben haben muss. Jedes Mal, wenn Sie einen Auftragnehmer hinzuziehen müssen, um die Leistung zu "reparieren", ist dies ein Zeichen für sehr schlecht geschriebenen Code oder ein sehr komplexes Problem, das High-End-Expertise erfordert.
Weiter: Wenn Sie sich beschweren, dass sie den Code-Stil geändert haben, obwohl Sie keinen Code-Stil hatten, machen Sie einfach einen sinnlosen Streit darüber, dass Ihr Mojo besser ist als der Mojo eines anderen. Vielleicht sollten Sie die Person fragen, warum sie Änderungen vorgenommen hat, die syntaktisch erscheinen, so dass Sie ein vollständiges Bild haben.
Ich betrachte die lange Liste einseitiger Antworten auf diesen Beitrag und frage mich, was mit der anderen Seite passiert ist. Leute nehmen die Emotion heraus und betrachten sie objektiv. Es ist oft erstaunlich, wie viele Leute über eine schöne Algorithmuslösung für ein komplexes Problem hinwegsehen werden, nur um zu bemerken, dass die Namenskonvention der Variablen von camel case in pascal case geändert wurde. Ich setze diese Art der Reaktion im Allgemeinen auf die Rechtfertigung des Selbstwerts zurück, indem ich immaterielle Fehler feststelle.
Die Schlüsselfrage, die ich stellen muss, lautet: Macht der neu formatierte Code die Anwendung weniger lesbar? Wenn Sie Budgeteinschränkungen hatten, warum haben Sie nicht explizit angegeben, dass Sie sehr spezifische Korrekturen wünschen und sonst nichts? Wenn Sie einen bestimmten Codierungsstil beibehalten möchten, warum haben Sie das nicht explizit angegeben?
Tags und Links c# coding-style