ist mein Code Standardkonform



  • ich möchte folgenden Code demnächst in Form eines Tutorials veröffentlichen - damit ich nicht etwas veröffentliche was nicht Standardkonform ist möchte ich euch mal bitten den Code anzusehen und mir mitzuteilen wo etwas zu bemängeln ist - PS: ich weiß das die Fkt SetPixel nicht Standardkonfrom ist 😉 - aber der rest sollte es schon sein

    void Bresenham(int d1x, int d1y, int d2x, int d2y) 
    {    
        bool X, Y, Z;        // zur Bestimmung der Bewegunsrichtung 
        int da, db;            // delta_a und delta_b 
        int *m;            // Bewegungsrichtung/Vektoren 
    
        // delta a, delta b und Bewegungsrichtung bestimmen 
        int dx = abs(d2x) - abs(d1x); 
        int dy = abs(d2y) - abs(d1y); 
    
        if(abs(dx)-abs(dy)>=0) 
        { 
            da = abs(dx); 
            db = abs(dy); 
            Z = true; 
        } 
        else 
        { 
            da = abs(dy); 
            db = abs(dx); 
            Z = false; 
        } 
    
        if(dx >= 0) 
            X = true; 
        else 
            X = false; 
    
        if(dy >= 0) 
            Y = true; 
        else 
            Y = false; 
    
        int array[8][4] = 
        { 
            { 0,-1,-1,-1 },        
            { 0,-1,1,-1 },        
            { 0,1,-1,1 },        
            { 0,1,1,1 },        
            { -1,0,-1,-1 },        
            { 1,0,1,-1 },        
            { -1,0,-1,1 },        
            { 1,0,1,1 },        
        }; 
    
        m = array[(X?1:0) + (Y?2:0) + (Z?4:0)]; 
    
        int gradient = (db<<1) - da; 
        int x = d1x; 
        int y = d1y; 
    
        SetPixel(x, y); 
    
        for(int i = 0; i < da; i++) 
        { 
            if(gradient >= 0) 
            { 
                x = x + m[2]; 
                y = y + m[3]; 
                gradient = gradient + (db<<1) - (da<<1); 
            } 
            else 
            { 
                x = x + m[0]; 
                y = y + m[1]; 
                gradient = gradient + (db<<1); 
            } 
            SetPixel(x, y); 
        } 
    }
    


  • Ich kann nichts Problembehaftetes erkennen.

    Wenn dir unsicher bist: Hol dir einen Compiler, der dich eigene Spracherweiterungen deaktivieren lässt.

    Ob es allerdings in einem Tutorial sinnvoll ist, einerseits:

    m = array[(X?1:0) + (Y?2:0) + (Z?4:0)];

    zu schreiben, und dann wieder:

    if(dx >= 0)
    X = true;
    else
    X = false;

    if(dy >= 0)
    Y = true;
    else
    Y = false;

    ist eine andre Frage 😉



  • In Anbetracht deines Tutorial sind mir doch ein paar Kleinigkeiten aufgefallen:

    1.) Initialisierung:

    bool X, Y, Z;        // zur Bestimmung der Bewegunsrichtung
        int da, db;            // delta_a und delta_b
        int *m;            // Bewegungsrichtung/Vektoren
    

    Wenn du die Variablen gesammelt definierst und nicht erst unmittelbar vor Gebrauch,
    dann solltest du sie zumindest initialisieren.

    2.) Variablennamen:
    - Gleiche Namen nur durch Gross-/Kleinschreibung zu unterscheiden (x,X) trägt
    nicht besonders zur Übersicht bei.
    - Ein Name wie array ist auch nicht sehr aussagekräftig.

    3.) In einem Tutorial kann hin und wieder ein Kommentar nicht schaden. Ohne
    den Funktionsnamen, denn ja nicht jeder kennen muss, hätt ich mir so recht
    nicht vorstellen können was das ganze überhaupt macht.



  • ausserdem ist der algo viel zu umständlich. hier eine version ohne multiplikationen und ohne lookup tables:

    void Circle (int x, int y, int radius)
    {
      int balance, xoff, yoff;
    
      xoff = 0;
      yoff = radius;
      balance = -radius;
    
      do 
      {
        SetPixel(x+xoff, y+yoff);
        SetPixel(x-xoff, y+yoff);
        SetPixel(x-xoff, y-yoff);
        SetPixel(x+xoff, y-yoff);
        SetPixel(x+yoff, y+xoff);
        SetPixel(x-yoff, y+xoff);
        SetPixel(x-yoff, y-xoff);
        SetPixel(x+yoff, y-xoff);
    
        if ((balance += xoff++ + xoff) >= 0)
          balance -= --yoff + yoff;
    
      } while (xoff <= yoff);
    }
    


  • net schrieb:

    balance += xoff++ + xoff
    balance -= --yoff + yoff;
    

    Beide Ausdrücke haben undefined behaviour und sind damit nicht mehr standardkonform.

    Der Standard sagt:

    Between the previous and next sequence point a scalar object shall have its stored value modified at most once by the evaluation of an expression. Furthermore, the prior value shall be accessed only to determine the value to be stored.

    Im ersten Fall wird xoff einmal modifiziert, aber es wird zweimal auf xoff zugegriffen. Beim zweiten Mal wird der Wert aber nicht verwendet, um den modifizierten Wert zu bestimmen.



  • Ponto schrieb:

    net schrieb:

    balance += xoff++ + xoff
    balance -= --yoff + yoff;
    

    Beide Ausdrücke haben undefined behaviour und sind damit nicht mehr standardkonform.

    ja, stimmt. das kommt davon wenn man einfach was von fremden webseiten kopiert 😡



  • Vertexwahn schrieb:

    if(dx >= 0) 
            X = true; 
        else 
            X = false; 
    
        if(dy >= 0) 
            Y = true; 
        else 
            Y = false;
    

    na dann mach doch lieber

    X = (dx >= 0);
    Y = (dy >= 0);
    

    das ist doch viel schöner und für den leser auch nicht schwerer zu verstehen. und weil du X und Y vorher noch nicht benutzt hast, mach doch lieber

    bool X = (dx >= 0);
    bool Y = (dy >= 0);
    

    und mach die deklerationen oben weg.

    dann ist da noch

    m = array[(X?1:0) + (Y?2:0) + (Z?4:0)];
    

    ich würd ja eher schreiben

    m = array[X + Y*2 + Z*4];
    


  • > außerdem ist der algo viel zu umständlich

    Augen auf beim Eierklau 😉
    mein Programm rastert Geradenstücke – deines einen Kreis

    > hier eine version ohne multiplikationen

    Mein Code arbeitet doch auch ohne Multiplikation – Bitshiften ist doch schneller als mit 2 multiplizieren

    > Wenn du die Variablen gesammelt definierst und nicht erst unmittelbar vor Gebrauch,
    dann solltest du sie zumindest initialisieren.

    Ich weiß 😉
    @Chew-Z:
    Du hast natürlich absolut recht – mein Code ist furchtbar – das ist mir durchaus bewusst – aber er ist auch mit 20 Seiten Dokumentiert 😉 – wenn man den Bresenham Algo verstanden hat ist der Programmcode leicht nachvollziehbar – wenn ich den Variablen schöne Namen verpasse, dann wird man den Algorithmus auch nicht nachvollziehen können. Dazu muss man sich einen Haufen Skizzen machen um das verstehen zu können (darum 20 Seiten ;))

    @borg:

    X?(1:0) + (Y?2:0) + (Z?4:0) != X + Y*2 + Z*4;
    

    Da kommen verschiednen Werte raus - die Klammern sind erforderlich - die kann man nicht einfach weglassen - (oder die zwei verwendeten Compiler mit dennen ich es getestet habe weichen vom C++ Standard ab)



  • Nach den vorgeschlagenen Änderungen sieht der Code jetzt wie folgt aus:

    void Bresenham(int d1x, int d1y, int d2x, int d2y)
    	{	
    		bool Z;			// zur Bestimmung der Bewegunsrichtung
    		int da, db;		// delta_a und delta_b
    		int *m;			// Bewegungsrichtung/Vektoren
    
    		// delta a, delta b und Bewegungsrichtung bestimmen 
    		int dx = abs(d2x) - abs(d1x);
    		int dy = abs(d2y) - abs(d1y);
    
    		if(abs(dx)-abs(dy)>=0)
    		{
    			da = abs(dx);
    			db = abs(dy);
    			Z = true;
    		}
    		else
    		{
    			da = abs(dy);
    			db = abs(dx);
    			Z = false;
    		}
    
    		bool X = (dx >= 0); 
    		bool Y = (dy >= 0);
    
    		int array[8][4] =
    		{
    			{ 0,-1,-1,-1 },		
    			{ 0,-1,1,-1 },		
    			{ 0,1,-1,1 },		
    			{ 0,1,1,1 },		
    			{ -1,0,-1,-1 },		
    			{ 1,0,1,-1 },		
    			{ -1,0,-1,1 },		
    			{ 1,0,1,1 },		
    		};
    
    		m = array[(X?1:0) + (Y?2:0) + (Z?4:0)];
    
    		int gradient = (db<<1) - da;
    		int x = d1x;
    		int y = d1y;
    
    		SetPixel(x, y, 255, 0, 0);
    
    		for(int i = 0; i < da; i++)
    		{
    			if(gradient >= 0)
    			{
    				x = x + m[2];
    				y = y + m[3];
    				gradient = gradient + (db<<1) - (da<<1);
    			}
    			else
    			{
    				x = x + m[0];
    				y = y + m[1];
    				gradient = gradient + (db<<1);
    			}
    
    			SetPixel(x, y, 255, 0, 0);
    		}
    	}
    


  • AFAIK ist nicht festgelegt, dass bool true == int 1 ist.



  • > AFAIK ist nicht festgelegt, dass bool true == int 1 ist.

    im C++ Kompendium steht:
    Der Integer Wert 0 wird als false interpretiert

    Ein Integer Wert ungleich 0 wird als true interpretiert

    Das Buch bezieht sich auf ISO/IEC 14882 von 1998(?)
    hat sich da was geändert?



  • Hast du mich falsch verstanden? Ich wollte sagen, dass bar hier nicht 1 sein muss:

    bool foo = true;
    int bar = foo;
    


  • wo spielt das in meinem Code eine Rolle?

    bool X = (dx >= 0); 
    bool Y = (dy >= 0);
    

    wird aus (dx >= 0) nicht automatisch ein boolscher Wert?

    PS: ja ich hab dich falsch verstanden - dachte da besteht eine Äquivalenzrelation



  • Das war ursprünglich für borg gedacht.



  • aso - wollte nur sichergehen das mein code standardkonform ist - nicht das ich etwas veröffenltiche was dann nicht mal von einem Standardkonformen C++ Compiler übersetzt werden kann - wäre schon peinlich



  • Ich denk mal, wenn Comeau es ohne Warnung kompiliert, wird es schon standardkonform genug sein. Google doch mal danach.



  • Vertexwahn schrieb:

    if(abs(dx)-abs(dy)>=0)
    		{
    			da = abs(dx);
    			db = abs(dy);
    			Z = true;
    		}
    		else
    		{
    			da = abs(dy);
    			db = abs(dx);
    			Z = false;
    		}
    

    das würde ich so schreiben:

    int da = abs(dx), db = abs(dy);
        if(z = da - db >= 0)
            swap(da, db);
    

    Und das geschifte ist auch unnötig (wenn nicht sogar unstandardkonform), das optimiert der Compiler schon.



  • @DrGreenthumb:
    Du hast einen kleinen Logikfehler gemacht
    Das Bitshiften hab ich drin gelassen - denk dir einfach eine Multiplikation mit 2 😉 - ich will mich jetzt nicht streiten - oder ist es wirklich so schlimm und ich sollte das lieber nicht machen?

    Der Code sieht jetzt so aus:

    void Bresenham(int d1x, int d1y, int d2x, int d2y) 
        {    
            bool Z = true;     // zur Bestimmung der Bewegunsrichtung 
            int *m;            // Bewegungsrichtung/Vektoren 
    
            // delta a, delta b und Bewegungsrichtung bestimmen 
            int dx = abs(d2x) - abs(d1x); 
            int dy = abs(d2y) - abs(d1y); 
    
    		int da = abs(dx), db = abs(dy); 
    
    		if(da-db<0)
    		{
    			int tmp = da;
    			da = db;
    			db = tmp;
    			Z = false;
    		}
    
    		bool X = (dx >= 0); 
            bool Y = (dy >= 0); 
    
            int array[8][4] = 
            { 
                { 0,-1,-1,-1 },        
                { 0,-1,1,-1 },        
                { 0,1,-1,1 },        
                { 0,1,1,1 },        
                { -1,0,-1,-1 },        
                { 1,0,1,-1 },        
                { -1,0,-1,1 },        
                { 1,0,1,1 },        
            }; 
    
            m = array[(X?1:0) + (Y?2:0) + (Z?4:0)]; 
    
            int gradient = (db<<1) - da; 
            int x = d1x; 
            int y = d1y; 
    
            SetPixel(x, y, 255, 0, 0); 
    
            for(int i = 0; i < da; i++) 
            { 
                if(gradient >= 0) 
                { 
                    x = x + m[2]; 
                    y = y + m[3]; 
                    gradient = gradient + (db<<1) - (da<<1); 
                } 
                else 
                { 
                    x = x + m[0]; 
                    y = y + m[1]; 
                    gradient = gradient + (db<<1); 
                } 
    
                SetPixel(x, y, 255, 0, 0); 
            } 
        }
    

    Der Code wird ja immer kürzer 😉 bald passt er auf eine Seite 😉



  • Bitshifts sind eigentlich unnötig geworden. Fast jeder Compiler optimiert das.


  • Mod

    Michael E. schrieb:

    Hast du mich falsch verstanden? Ich wollte sagen, dass bar hier nicht 1 sein muss:

    bool foo = true;
    int bar = foo;
    

    doch muss - stichwort integral promotions (4.5)

    An rvalue of type bool can be converted to an rvalue of type int, with false becoming zero and true becoming one.


Anmelden zum Antworten