CODEWITHSUNDEEP
javaclass
Marian Pazioch
Marian Pazioch

Reputation: 83

General programming - usage of classes

Ok, recently i have written a "paint application" in Java. Small program similar to MS Windows Paint and apparently (according to some people that evaluated my source code) im not using classes properly in my application. Unfortunatelly i have no idea what could be wrong, and yet noone is in the mood to tell me what is incorrect.

I have "proper" classes that use the idea of inheritance:

PaintObject  
-ClosedObject  
--RectangleShape  
--OvalShape  
-OpenObject  
--PointShape  
--LineShape

But still i have no idea what could be wrong in code, what could lead to thinking that classes are not used properly...

The only thing what (in my opinion) could be wrong is the fact that im drawing by creating the new objects like: new RectagleObject(...) and new OvalShape(...) instead of creating one field objectToBeDrawn variable (maybe a PaintObject type) and then assign appropriate shape to it when the method executeOperation is called (obviously along with drawing of that particular shape)

Here is the code sample:

public void executeOperation(Graphics2D gr, int oper, boolean drawingMode){
    if(oper==1){
        new RectangleShape(startLocation, endLocation, borderColor, fillColor).draw(gr);
    }
    else if(oper==2){
        new OvalShape(startLocation, endLocation, borderColor, fillColor).draw(gr);
    }
    ....//more operations
}

Anyone has any idea what could be wrong in terms of general programming habits (unless my 'guess' is right? - if it is, please confirm my doubts). Or maybe there is something else? (Skip the part of oper==1 since i know that such values should be defined constant)

Any help greatly appreciated, im learning by myself and it is really hard to guess how professional programs are designed (and follow their pattern) since my experience is "limited" (actually none).

Upvotes: 1

Views: 138

Answers (4)

Zong
Zong

Reputation: 6230

If you're drawing raster images, do you need to keep track of what the user is drawing? Perhaps you mean objects such as PenTool, RectangleTool, etc?

Right now, it seems like you create objects that persist and are redrawn to draw specific shapes. You can simply have the tool objects draw on an Image once when appropriate (just don't ever clear the graphics context). Just assign a particular tool to some activeTool field when the tool is selected. 

class DrawingTool {
     public void draw(Graphics g) { }
}

class PenTool extends DrawingTool {
     public void draw(Graphics g) {
         // some drawing logic
     }
}

class RectangleTool extends DrawingTool {
     public void draw(Graphics g) {
         // some other drawing logic
     }
}

So instead of

oper = 1;

you would write

activeTool = new PenTool(); // or activeTool = PenTool.GetInstance();

or something similar. Now when you want to draw something, just invoke the draw method. As I stated before, you can do this simply by passing in an Image that you never clear: 

class DrawingCanvas {
     Image image = createImage(GetWidth(), GetHeight());
     DrawingTool activeTool;

     // this is what gets invoked on every frame
     public void paint(Graphics g){
          g.drawImage(image, 0, 0, null);
     }
}

// when you need to draw something
// (not on every frame, but something new)
activeTool.draw(image.getGraphics());

Upvotes: 3

Localghost
Localghost

Reputation: 712

Each of your types (specifically OvalShape and RectangleShape) should both implement an "executeOperation" method. This method should override the "executeOperation" method in the base class (closed object).

This will get rid of the need to if(oper == 2)... else (oper == 2). It will just call the implementation that is relevant for the object's type.

Upvotes: 1

StriplingWarrior
StriplingWarrior

Reputation: 156708

if(oper==1){
    //...
}
else if(oper==2){
    //...
}

This is the sort of code that proper use of Object-Oriented programming should be able to avoid 99% of the time. Investigate why you need an oper integer at all. There's probably a pattern you can modify to eliminate the need for it. For example, if you're currently assigning an integer value to each button in the toolbar, you might instead make the button handler set the current OperationFactory to one that produces the object of the type associated with that button.

Upvotes: 3

squishy
squishy

Reputation: 61

from the small snippet you could probably make more use of polymorphism. like have a parent abstract class/interface that defines the executeOperation(Graphics2D gr, boolean drawingMode) method then extend/implement it for each operation you support (may be able to drop drawingmode too i can't see what it does though.) Can possibly put the graphics2D into a constructor too.

Upvotes: 0

Related Questions