Jaki był najgorszy fragment kodu, który naprawiłeś, z którego jesteś dumny? [Zamknięte]


14

Mam kilka, z których jestem dumny, a niektóre z nich napisałem sam kilka lat temu. To niekoniecznie musi być błędny, tylko zły kod.


8
Zbyt wielu do opublikowania, czy mogę po prostu przekazać Ci moją historię SVN? ;)
Nicole

2
Znasz stronę thedailywtf.com ?

Odpowiedzi:


20

Nie wiem, czy jestem dumny z tej poprawki, ponieważ była ona tak oczywista, ale najbardziej przerażającym kodem, jaki pamiętam, była poprawka.

if (userName=="John O'Reily") { userName= "John O''Reily";}
if (userName=="Stacy O'Neil") { userName= "Stacy O''Neil";}
if (userName=="Finnegan O'Connor") { userName= "Finnegan O''Connor";}
...
someSQL = "SELECT * from Users where UserName='" + userName + "'";

Najwyraźniej poprzedni programista po prostu dodawał nowe wiersze za każdym razem, gdy nowy (zwykle irlandzki) użytkownik zaczął otrzymywać błędy w aplikacji.

Zostawię to jako ćwiczenie dla klasy, jak to zostało naprawione.


Biorę SQL konkatenuje dwa sąsiednie literały łańcuchowe, prawda?
Amarghosh,

1
@Amarghosh: Huh?
JohnFx,

2
@Amarghosh: to ucieczka. Podwójne pojedyncze cudzysłowy redukują się do pojedynczego cudzysłowu wewnątrz literału ciągowego.
Mason Wheeler,

Dzięki @Mason, o którym myślałem, UserName='John O''Reily'że się stanie UserName='John OReily'(tak jak C łączy sąsiednie literały łańcuchowe), ale nie pomyślałem o brakującym ':(
Amarghosh

1
@JohnFx - Zgaduję, że poprawka zmieniła oryginalną instrukcję SQL w sparametryzowany SQL, aby wyeliminować wstrzykiwanie SQL?
Melioratus

7

Nie powinienem być z tego dumny, ale z jakiegoś powodu było to satysfakcjonujące.

Poza tym, że mam COBOL w szkole, nie miałem doświadczenia, ale byłem nisko człowiekiem na słupie totemowym i musieliśmy dostarczyć kompilatorowi kod źródłowy do outsourcera w celu sprawdzenia Y2K. Mieliśmy jeden plik COBOL z wieloma procedurami, które wywoływały się w nim, przypominając spaghetti, i był zbyt duży, aby załadować go w naszym obecnym IDE do skompilowania. Musiał zostać podzielony na co najmniej dwa pliki fizyczne, a te pliki oczywiście musiały mieć wszystko, co potrzebne w swoim własnym pliku. (A może istniał sposób na połączenie ich ze sobą, ale tak naprawdę nie znałem języka COBOL.)

W każdym razie wziąłem ten plik z około 100 000 wierszy i delikatnie rozdzieliłem dziesiątki procedur, aby znaleźć dwa zestawy procedur, które były od siebie niezależne i dlatego mogły istnieć w dwóch osobnych plikach, każdy około 50 000 linii. (Myślę, że maksimum, które kompilator mógł obsłużyć, to około 80 000 linii, więc musiało być dość równomiernie dopasowane).

Czytałem starożytny język, którego nie znałem i nadal byłem w stanie wykonać zadanie.


5
W ten sposób analizowałeś skamielinę napisaną u dinozaura. Lub przeprowadzanie operacji na skale. Lub ... moja analogia się rozpadła. Wsparcie!
Jon Purdy,

2
Jeszcze jedno dla czytelności COBOL!

Jakie było IDE, które miało limit linii?
Wieża

Jesteśmy sklepem Cobol i istnieje jedno ograniczenie z jednym z narzędzi (kompilator lub edytor, nie jestem pewien) z 65535 liniami. Może to być coś podobnego.

6

Wyjąłem kursor ze spustu i skróciłem czas na wstawienie 40 000 nowych rekordów z godziny do mniej niż minuty. Ostatecznie spowodowało to, że mogłem wstawić 21 milionów rekordów w czasie krótszym niż lodowaty, ale nigdy nie próbowaliśmy importu 20 milionów rekordów do czasu poprawki, więc nie mam statystyk dotyczących tego, ile czasu zaoszczędziliśmy.


2
I oczywiście ktoś w marketingu żądał uznania za przyspieszenie? :-)
Tin Man

4

Była podstawowa klasa do tworzenia okien dialogowych potwierdzenia dla różnych operacji na węzłach drzewa. Wystarczy, że podasz komunikat do wyświetlenia w oknie dialogowym, a akcja zostanie uruchomiona, jeśli zostanie potwierdzona. Niezły system, ale nie pozwalał na specjalną obsługę w przypadku, gdyby nie wybrano węzła drzewa. W rezultacie tekst w jednym z okien dialogowych brzmiał: „Proszę wybrać nie”. Jeśli wybierzesz opcję tak, zgłosił wyjątek. Rzeczywiście bardzo przyjemna obsługa.

Naprawiłem to, wyłączając nieprawidłowe operacje.


1
W takim razie powinienem wybrać „nie”!
Alan Pearce

2

Najgorsze, co widziałem, to kod Java do wydobywania kluczowych zdań z korpusu tekstowego.

  • Kod miał kilka komentarzy (oprócz wyłączonych komentarzy do kodu) i miał kiepskie nazwy.
  • Stan algorytmu był przechowywany w publicznych wektorach statycznych.
  • Zamiast przechowywać wartości w wektorach, przechowywał ich reprezentacje łańcuchowe.
  • Wektory były klasami ubogich (były równoległe, każdy indeks był „instancją”)
  • Algorytm nie był optymalny (n ^ 2 zamiast łatwiejszej do zrozumienia wersji n log n)

Szczerze mówiąc, to nic w porównaniu z niektórymi z naszych rzeczy, ale wciąż istnieje ogromna różnica w jakości przed i po. Rozważ następujące rzeczywiste kody przed i po jednej funkcji:

Przed (spróbuj dowiedzieć się, co robi, zanim spojrzysz na After!):

public static void getCluster() {
    count = new Vector();
    for (int i = 0; i < begin.size(); i ++)
        count.add("1");
    if (begin.size() > 1) {
        for (int i = 0; i < begin.size() - 1; i ++) {
            for (int j = i + 1; j < begin.size(); j ++) {
                int b1 = Integer.parseInt(begin.get(i).toString());
                int e1 = Integer.parseInt(end.get(i).toString());
                double w1 = Double.parseDouble(wght.get(i).toString());
                int c1 = Integer.parseInt(count.get(i).toString());
                int b2 = Integer.parseInt(begin.get(j).toString());
                int e2 = Integer.parseInt(end.get(j).toString());
                double w2 = Double.parseDouble(wght.get(j).toString());
                int c2 = Integer.parseInt(count.get(j).toString());
                int max = Math.max(e1, e2);
                boolean toRemove = true;
                if (b1 == b2) end.set(i, Integer.toString(max));
                if (b1 < b2) {
                    if (b2 < e1) end.set(i, Integer.toString(max));
                    else {
                        if ((b2 - e1) <= 3) end.set(i, Integer.toString(e2));
                        else toRemove = false;
                    }
                }
                if (b1 > b2) {
                    if (e2 >= b1) {
                        begin.set(i, Integer.toString(b2));
                        end.set(i, Integer.toString(max));
                    } else {
                        if ((b1 - e2) <= 3) {
                            begin.set(i, Integer.toString(b2));
                            end.set(i, Integer.toString(e1));
                        } else toRemove = false;
                    }
                }
                //System.out.println(b1 + ", " + e1 + ", " + b2 + ", " + e2 + " ---> " + begin.get(i).toString() + ", " + end.get(i).toString());
                if (toRemove) {
                    wght.set(i, Double.toString(w1 + w2));
                    count.set(i, Integer.toString(c1 + c2));
                    begin.removeElementAt(j);
                    end.removeElementAt(j);
                    wght.removeElementAt(j);
                    count.removeElementAt(j);
                    j --;
                } //end of if
            } //end of for j
        } //end of for i
    } //end of if
    //System.out.println(begin);
    //System.out.println(end);
    //System.out.println(wght);
    //System.out.println(count);
}

Po:

/** Returns the result of merging all overlapping-with-leeway clusters into single combined clusters.
 * @param leeway The minimum number of word-spaces which must separate two clusters in order for them to not be overlapping.
 * @requires clusters != null
 * @requires leeway >= 0
 * @ensures result != null */ 
private static List<TermCluster> combineOverlappingClusters(Iterable<TermCluster> clusters, int leeway) {
    if (clusters == null) throw new NullPointerException("clusters");
    if (leeway < 0) throw new IllegalArgumentException("leeway < 0");

    //Sort to allow linear folding
    List<TermCluster> sortedClusters = Iter.sort(clusters, new Comparator<TermCluster>() {
        @Override public int compare(TermCluster o1, TermCluster o2) {
            return new Integer(o1.begin).compareTo(o2.begin);
        }
    });
    if (sortedClusters.size() == 0)
        return sortedClusters;

    //Combine left-to-right
    List<TermCluster> result = new ArrayList<TermCluster>();
    TermCluster acc = sortedClusters.get(0);
    for (TermCluster cluster : sortedClusters.subList(1, sortedClusters.size())) {
        if (acc.isOverlappingWithLeeway(cluster, leeway)) {
            //combine
            acc = acc.combineWith(cluster);
        } else {
            //next
            result.add(acc);
            acc = cluster;
        }            
    }
    result.add(acc); //leftovers

    return result;
}

1

Moim pierwszym zadaniem programistycznym było pisanie instalatorów w InstallShield. Odziedziczyłem skrypt, który składał się z tysięcy linii bez żadnych funkcji , tylko gotos. To było oszałamiające. Przepisałem go, uczyniłem wszystko ładnym, modułowym i opartym na danych, tak że mogłem otrzymać pliki binarne / art / etc. i okazuje się, że nowy instalator zajmuje mniej niż godzinę, a nie tydzień + zajęło to poprzedniemu facetowi. Byłem z siebie bardzo dumny.


0

Myślę, że nic nie zbliża się do tego :

function pmn_Sort(strBy)

local tblA = {};
local tblB = {};
local tblC = {};
local tblD = {};
local tblE = {};
local tblF = {};
local tblG = {};
local tblH = {};
local tblI = {};
local tblJ = {};
local tblK = {};
local tblL = {};
local tblM = {};
local tblN = {};
local tblO = {};
local tblP = {};
local tblQ = {};
local tblR = {};
local tblS = {};
local tblT = {};
local tblU = {};
local tblV = {};
local tblW = {};
local tblX = {};
local tblY = {};
local tblZ = {};
local tblOT = {};

local iA = 0;
local iB = 0;
local iC = 0;
local iD = 0;
local iE = 0;
local iF = 0;
local iG = 0;
local iH = 0;
local iI = 0;
local iJ = 0;
local iK = 0;
local iL = 0;
local iM = 0;
local iN = 0;
local iO = 0;
local iP = 0;
local iQ = 0;
local iR = 0;
local iS = 0;
local iT = 0;
local iU = 0;
local iV = 0;
local iW = 0;
local iX = 0;
local iY = 0;
local iZ = 0;
local iOT = 0;

    if strBy == "Name" then

        strSort = "Name";
        numPlcount = ListBox.GetCount("Playlist");
        numPLitem = 1;
        numPLadd = 0;
        while numPLitem &lt;= numPlcount do

            strPLtxt = ListBox.GetItemText("Playlist", numPLitem);
            strPLdata = ListBox.GetItemData("Playlist", numPLitem);
            strPLleft = String.Left(strPLtxt, 1);
            if strPLleft == "a" or strPLleft == "A" then

            iA = iA + 1;
            tblA[iA] = strPLdata;

            elseif strPLleft == "b" or strPLleft == "B" then

            iB = iB + 1;
            tblB[iB] = strPLdata;

            elseif strPLleft == "c" or strPLleft == "C" then

            iC = iC + 1;
            tblC[iC] = strPLdata;

            elseif strPLleft == "d" or strPLleft == "D" then

            iD = iD + 1;
            tblD[iD] = strPLdata;

            elseif strPLleft == "e" or strPLleft == "E" then

            iE = iE + 1;
            tblE[iE] = strPLdata;

            elseif strPLleft == "f" or strPLleft == "F" then

            iF = iF + 1;
            tblF[iF] = strPLdata;

            elseif strPLleft == "g" or strPLleft == "G" then

            iG = iG + 1;
            tblG[iG] = strPLdata;

            elseif strPLleft == "h" or strPLleft == "H" then

            iH = iH + 1;
            tblH[iH] = strPLdata;

            elseif strPLleft == "i" or strPLleft == "I" then

            iI = iI + 1;
            tblI[iI] = strPLdata;

            elseif strPLleft == "j" or strPLleft == "J" then

            iJ = iJ + 1;
            tblJ[iJ] = strPLdata;

            elseif strPLleft == "k" or strPLleft == "K" then

            iK = iK + 1;
            tblK[iK] = strPLdata;

            elseif strPLleft == "l" or strPLleft == "L" then

            iL = iL + 1;
            tblL[iL] = strPLdata;

            elseif strPLleft == "m" or strPLleft == "M" then

            iM = iM + 1;
            tblM[iM] = strPLdata;

            elseif strPLleft == "n" or strPLleft == "N" then

            iN = iN + 1;
            tblN[iN] = strPLdata;

            elseif strPLleft == "o" or strPLleft == "O" then

            iO = iO + 1;
            tblO[iO] = strPLdata;

            elseif strPLleft == "p" or strPLleft == "P" then

            iP = iP + 1;
            tblP[iP] = strPLdata;

            elseif strPLleft == "q" or strPLleft == "Q" then

            iQ = iQ + 1;
            tblQ[iQ] = strPLdata;

            elseif strPLleft == "r" or strPLleft == "R" then

            iR = iR + 1;
            tblR[iR] = strPLdata;

            elseif strPLleft == "s" or strPLleft == "S" then

            iS = iS + 1;
            tblS[iS] = strPLdata;

            elseif strPLleft == "t" or strPLleft == "T" then

            iT = iT + 1;
            tblT[iT] = strPLdata;

            elseif strPLleft == "u" or strPLleft == "U" then

            iU = iU + 1;
            tblU[iU] = strPLdata;

            elseif strPLleft == "v" or strPLleft == "V" then

            iV = iV + 1;
            tblV[iV] = strPLdata;

            elseif strPLleft == "w" or strPLleft == "W" then

            iW = iW + 1;
            tblW[iW] = strPLdata;

            elseif strPLleft == "x" or strPLleft == "X" then

            iX = iX + 1;
            tblX[iX] = strPLdata;

            elseif strPLleft == "y" or strPLleft == "Y" then

            iY = iY + 1;
            tblY[iY] = strPLdata;

            elseif strPLleft == "z" or strPLleft == "Z" then

            iZ = iZ + 1;
            tblZ[iZ] = strPLdata;

            else

            iOT = iOT + 1;
            tblOT[iOT] = strPLdata;

            end

            numPLitem = numPLitem + 1;

        end

        ListBox.DeleteItem("Playlist", LB_ALLITEMS);

        for ii, id in tblA do

            if id ~= "" then

                numPLadd = numPLadd + 1;
                strPLtxt = String.SplitPath(id).Filename..String.SplitPath(id).Extension
                ListBox.AddItem("Playlist", strPLtxt, id);

            end

        end

        for ii, id in tblB do

            if id ~= "" then

                numPLadd = numPLadd + 1;
                strPLtxt = String.SplitPath(id).Filename..String.SplitPath(id).Extension
                ListBox.AddItem("Playlist", strPLtxt, id);

            end

        end

        for ii, id in tblC do

            if id ~= "" then

                numPLadd = numPLadd + 1;
                strPLtxt = String.SplitPath(id).Filename..String.SplitPath(id).Extension
                ListBox.AddItem("Playlist", strPLtxt, id);

            end

        end

        for ii, id in tblD do

            if id ~= "" then

                numPLadd = numPLadd + 1;
                strPLtxt = String.SplitPath(id).Filename..String.SplitPath(id).Extension
                ListBox.AddItem("Playlist", strPLtxt, id);

            end

        end

        for ii, id in tblE do

            if id ~= "" then

                numPLadd = numPLadd + 1;
                strPLtxt = String.SplitPath(id).Filename..String.SplitPath(id).Extension
                ListBox.AddItem("Playlist", strPLtxt, id);

            end

        end

        for ii, id in tblF do

            if id ~= "" then

                numPLadd = numPLadd + 1;
                strPLtxt = String.SplitPath(id).Filename..String.SplitPath(id).Extension
                ListBox.AddItem("Playlist", strPLtxt, id);

            end

        end

        for ii, id in tblG do

            if id ~= "" then

                numPLadd = numPLadd + 1;
                strPLtxt = String.SplitPath(id).Filename..String.SplitPath(id).Extension
                ListBox.AddItem("Playlist", strPLtxt, id);

            end

        end

        for ii, id in tblH do

            if id ~= "" then

                numPLadd = numPLadd + 1;
                strPLtxt = String.SplitPath(id).Filename..String.SplitPath(id).Extension
                ListBox.AddItem("Playlist", strPLtxt, id);

            end

        end

        for ii, id in tblI do

            if id ~= "" then

                numPLadd = numPLadd + 1;
                strPLtxt = String.SplitPath(id).Filename..String.SplitPath(id).Extension
                ListBox.AddItem("Playlist", strPLtxt, id);

            end

        end

        for ii, id in tblJ do

            if id ~= "" then

                numPLadd = numPLadd + 1;
                strPLtxt = String.SplitPath(id).Filename..String.SplitPath(id).Extension
                ListBox.AddItem("Playlist", strPLtxt, id);

            end

        end

        for ii, id in tblK do

            if id ~= "" then

                numPLadd = numPLadd + 1;
                strPLtxt = String.SplitPath(id).Filename..String.SplitPath(id).Extension
                ListBox.AddItem("Playlist", strPLtxt, id);

            end

        end

        for ii, id in tblL do

            if id ~= "" then

                numPLadd = numPLadd + 1;
                strPLtxt = String.SplitPath(id).Filename..String.SplitPath(id).Extension
                ListBox.AddItem("Playlist", strPLtxt, id);

            end

        end

        for ii, id in tblM do

            if id ~= "" then

                numPLadd = numPLadd + 1;
                strPLtxt = String.SplitPath(id).Filename..String.SplitPath(id).Extension
                ListBox.AddItem("Playlist", strPLtxt, id);

            end

        end

        for ii, id in tblN do

            if id ~= "" then

                numPLadd = numPLadd + 1;
                strPLtxt = String.SplitPath(id).Filename..String.SplitPath(id).Extension
                ListBox.AddItem("Playlist", strPLtxt, id);

            end

        end

        for ii, id in tblO do

            if id ~= "" then

                numPLadd = numPLadd + 1;
                strPLtxt = String.SplitPath(id).Filename..String.SplitPath(id).Extension
                ListBox.AddItem("Playlist", strPLtxt, id);

            end

        end

        for ii, id in tblP do

            if id ~= "" then

                numPLadd = numPLadd + 1;
                strPLtxt = String.SplitPath(id).Filename..String.SplitPath(id).Extension
                ListBox.AddItem("Playlist", strPLtxt, id);

            end

        end

        for ii, id in tblQ do

            if id ~= "" then

                numPLadd = numPLadd + 1;
                strPLtxt = String.SplitPath(id).Filename..String.SplitPath(id).Extension
                ListBox.AddItem("Playlist", strPLtxt, id);

            end

        end

        for ii, id in tblR do

            if id ~= "" then

                numPLadd = numPLadd + 1;
                strPLtxt = String.SplitPath(id).Filename..String.SplitPath(id).Extension
                ListBox.AddItem("Playlist", strPLtxt, id);

            end

        end

        for ii, id in tblS do

            if id ~= "" then

                numPLadd = numPLadd + 1;
                strPLtxt = String.SplitPath(id).Filename..String.SplitPath(id).Extension
                ListBox.AddItem("Playlist", strPLtxt, id);

            end

        end

        for ii, id in tblT do

            if id ~= "" then

                numPLadd = numPLadd + 1;
                strPLtxt = String.SplitPath(id).Filename..String.SplitPath(id).Extension
                ListBox.AddItem("Playlist", strPLtxt, id);

            end

        end

        for ii, id in tblU do

            if id ~= "" then

                numPLadd = numPLadd + 1;
                strPLtxt = String.SplitPath(id).Filename..String.SplitPath(id).Extension
                ListBox.AddItem("Playlist", strPLtxt, id);

            end

        end

        for ii, id in tblV do

            if id ~= "" then

                numPLadd = numPLadd + 1;
                strPLtxt = String.SplitPath(id).Filename..String.SplitPath(id).Extension
                ListBox.AddItem("Playlist", strPLtxt, id);

            end

        end

        for ii, id in tblW do

            if id ~= "" then

                numPLadd = numPLadd + 1;
                strPLtxt = String.SplitPath(id).Filename..String.SplitPath(id).Extension
                ListBox.AddItem("Playlist", strPLtxt, id);

            end

        end

        for ii, id in tblX do

            if id ~= "" then

                numPLadd = numPLadd + 1;
                strPLtxt = String.SplitPath(id).Filename..String.SplitPath(id).Extension
                ListBox.AddItem("Playlist", strPLtxt, id);

            end

        end

        for ii, id in tblY do

            if id ~= "" then

                numPLadd = numPLadd + 1;
                strPLtxt = String.SplitPath(id).Filename..String.SplitPath(id).Extension
                ListBox.AddItem("Playlist", strPLtxt, id);

            end

        end     

        for ii, id in tblZ do

            if id ~= "" then

                numPLadd = numPLadd + 1;
                strPLtxt = String.SplitPath(id).Filename..String.SplitPath(id).Extension
                ListBox.AddItem("Playlist", strPLtxt, id);

            end

        end

        for ii, id in tblOT do

            if id ~= "" then

                numPLadd = numPLadd + 1;
                strPLtxt = String.SplitPath(id).Filename..String.SplitPath(id).Extension
                ListBox.AddItem("Playlist", strPLtxt, id);

            end

        end

    elseif strBy == "Type" then

        strSort = "Type";

        if File.DoesExist(_ProgramFilesFolder.."\\MediaX\\playlist.mx") == true then

            play_file2 = TextFile.ReadToTable(_ProgramFilesFolder.."\\MediaX\\playlist.mx");

            if play_file2 then
                ListBox.DeleteItem("Playlist", -1);

                for rl,rPath in play_file2 do
                    r2Path = String.TrimLeft(rPath, nil);
                    ListBox.AddItem("Playlist", String.SplitPath(r2Path).Filename..String.SplitPath(r2Path).Extension, r2Path);
                end
            end
        end
    end
end

Poprawka? Ech, to nie powinno wymagać wielu wyjaśnień.

Korzystając z naszej strony potwierdzasz, że przeczytałeś(-aś) i rozumiesz nasze zasady używania plików cookie i zasady ochrony prywatności.
Licensed under cc by-sa 3.0 with attribution required.