Ausnahmesicherheit, RAII, sicherer Code

Dobi

C++ Ausnahmesicherheit schrieb:

Da ich gerade beim C++ lernen bin, bin ich auch über sonstiges Feedback dankbar!

Na gut.
Falls du nicht gezwungen bist, aus irgendwelchen Gründen die veraltete C-Schnittstelle von OpenCV zu benutzen, schau dir mal die moderne C++-Schnittstelle an. Damit geht vieles wesentlich einfacher und du musst dir nicht um so viel Kram (Speicherfreigabe usw.) Gedanken machen. Gut dokumentiert ist sie ( http://docs.opencv.org/ ), und es gibt auch nette Tutorials ( http://docs.opencv.org/doc/tutorials/tutorials.html ).

Gruß
Dobi

krümelkacker

So richtig toll sieht das nicht aus, um ehrlich zu sein. Deine Klasse versucht hier gleich mehrere Resourcen direkt nach folgendem Muster

class Klasse
{
  int* a;
  int* b;
public:
  :::
  Klasse(Klasse const&);
  Klasse& operator=(Klasse const&);
  ~Klasse();
  :::
};
:::
Klasse::~Klasse()
{
  delete[] a;
  delete[] b;
}

zu verwalten. Das ist keine besonders gute Idee, da es die korrekte Implementierung von Klasse unnötig kompliziert macht. Wenn Du stattdessen eine Klasse für "dazwischen" baust (wie es z.B. std::vector<> hier wäre), dann vereinfacht es die Sache gewaltig:

class Klasse
{
  vector<int> a;
  vector<int> b;
public:
  ::: // selbstdefinierter copy-ctor, copy-assign und dtor sind unnötig
};

Das heißt, ich würde mir wahrscheinlich eine vernünftige Abstraktion für ein Bild machen, statt überall stellvertretend Zeiger dafür zu verwenden, welche explizites Resourcenmanagement erfordern.

Ggf wird damit Deine Klasse auch überflüssig. Ich wüsste jedenfalls nicht, wozu ein "ImageProcessingTool" einen Zustand benötigt. Aber Du hast Dir ja dabei irgend etwas gedacht ...

Und warum ist dein loadImageFromFile so kompliziert? Reicht da nicht auch folgendes?

void ImageProcessingTool::loadImageFromFile(const char *fileName) 
    { 
        ImageProcessingTool neu (fileName);
        this->swap(neu);
    }

Diesen Konstruktor musst Du dann natürlich ohne loadImageFromFile implementieren.

Und weil ich noch etwas von "base class" lese: Bist Du Dir sicher, dass Du Dir das mit der Vererbungshierarchie richtig überlegt hast?

Danke!

@Dobi: Die C++ Schnittstelle werd ich schnellstmöglich einbauen in mein Programm, dann entfällt schon mal diese blöde Speicherverwaltung.

@ich bins: das stimmt in dem Fall so, weil ich immer vom Ergebnis des letzten Objekts weiterarbeiten möchte. Also z.B. Bild rotieren->Kanten erkennen-> usw...

@krümelkacker: Die Logik also in den Konstruktor statt in loadImageFromFile. Jedenfalls wird loadImageFromFile dann sehr kurz, gefällt mir.
Die Zeiger entfallen, wenn ich die C++ Schnittstelle von OpenCV (siehe 2. Antwort) verwende.
Die von dir angesprochene Klassenhierachie sieht wie folgt aus, Feedback ist erwünscht:

(grundsätzlich möchte ich verschiedene Klassen zur Bildverarbeitung haben, z.B. rotieren, Kanten erkennen, Weichzeichnen, .... Und all diese Klassen haben eine gewisse Schnittmenge, die ich in die Basisklasse packe)

Basisklasse
ImageProcessingTool: stellt alles wichtige zur Verfügung wie z.B. die Bilder ansich, verschiedene Funktionen wie Bild laden, Bild auf Bildschirm anzeigen, usw...

Abgeleitete Klassen
RotationTool, EdgeDetectionTool, HistogramTool,...
bauen auf der grundlegenden Logik der Basisklasse auf. Man kann ja sagen Rotation Tool ist ein ImageProcessingTool, d.h. sollte eine public Ableitung hier passen. Sie erweitern die Basisklasse meist nur um ein oder zwei Funktionen, in denen die mathematischen Formeln abgearbeitet werden.

Wie schon gesagt, Feedback zur Klassenhierachie ist erwünscht.

Dobi

Ohne zu wissen, wofür du so eine Hierachie benötigst, frage ich einfach mal ins blaue: Ist das überhaupt nötig? Rotieren, Weichzeichnen und sonstige Geschichten sehen für mich erstmal nicht nach irgendwas aus, dass eine Klasse sein müsste, sondern mehr nach freien Funktionen.
Vielleicht lässt sich die Verwende-die-Ergebnisse-vom-vorherigen-Bild-Sache ja eleganter umsetzen als mit Stati in den (eventuell überflüssigen) Bearbeiter-Klassen. Wenn du deine Bearbeiter-Funktion irgendwie so aufrufen kannst, dass sie beliebige zwei Bilder (nicht nur Vorgänger und Nachfolger) nehmen kann, wäre sie universeller und du könntest von außen beliebig aufrufen. Vielleicht musst du ja aber auch nur die Ergebnisse von zwei Bearbeitungen zusammenbringen. Dann könntest du das Erzeugen der Ergebnisse und das Zusammenbringen ganz voneinander trennen. Was soll deine Anwendung eigentlich machen?

krümelkacker

Habe ich mir schon gedacht. Ich sehe hier keinen Sinn in der Verklassifizierung eines Algorithmus. Zumindest hast Du noch nicht erklärt, welchen Nutzen Du Dir davon versprichst.

Wie schon gesagt bin ich beim C++ lernen, d.h. der Hauptgrund ist dass ich auch Dinge wie Klassen und Ableitungen lerne. Ansonsten hätte ich natürlich die Funktionen auch direkt in die Basisklasse schmeißen können.

Nebenbei muss ich mir gerade (zumindest die Grundlagen) der Bildverarbeitung beibringen. OpenCV verwende ich erstmals nur für das laden von Bildern und zur Bereitstellung geeigneter Datenstrukturen. Auf diesen Datenstrukturen wende ich dann verschiedene (selbst programmierte) Bildbearbeitungsalgorithmen an.

Das ganze soll also eher ein Projekt sein, bei welchem ich möglichst viele Möglichkeiten von C++ "in der Praxis" ausprobieren kann.

Dobi schrieb:

Ohne zu wissen, wofür du so eine Hierachie benötigst, frage ich einfach mal ins blaue: Ist das überhaupt nötig? Rotieren, Weichzeichnen und sonstige Geschichten sehen für mich erstmal nicht nach irgendwas aus, dass eine Klasse sein müsste, sondern mehr nach freien Funktionen.
Vielleicht lässt sich die Verwende-die-Ergebnisse-vom-vorherigen-Bild-Sache ja eleganter umsetzen als mit Stati in den (eventuell überflüssigen) Bearbeiter-Klassen. Wenn du deine Bearbeiter-Funktion irgendwie so aufrufen kannst, dass sie beliebige zwei Bilder (nicht nur Vorgänger und Nachfolger) nehmen kann, wäre sie universeller und du könntest von außen beliebig aufrufen. Vielleicht musst du ja aber auch nur die Ergebnisse von zwei Bearbeitungen zusammenbringen. Dann könntest du das Erzeugen der Ergebnisse und das Zusammenbringen ganz voneinander trennen. Was soll deine Anwendung eigentlich machen?

Ok, hab noch ein bisschen über deine Sätze nachgedacht.
Wie sieht deiner Meinung nach das ideale Design für eine solche Software aus - sodass ich mich in der Zukunft daran orientieren kann.
Gehen wir von einer simplen Bildverarbeitungssoftware aus. Sie soll einerseits Bilder laden/speichern können und andererseits diese Bilder bearbeiten können.

Hast du das in etwa so gemeint:
+Klasse welche ein Bild speichert sowie Lade/Speicherfunktionen zur Verfügung stellt. Nennen wir sie mal BildKlasse
+freie Funktionen, welche als Argument/Rückgabetyp die oben beschriebene BildKlasse verwenden

Wie sieht die Implementierung dann idealerweise aus? Eine cpp/hpp für die Bildklasse, eine cpp/hpp für die freien Funktionen?
Oder die Funktionen als statische Funktionen doch in die Bildklasse aufnehmen?
Lass(t) mal deine (eure) Vorschläge hören!

krümelkacker

Guck Dir mal an, wie die STL funktioniert ... speziell die Sache mit den Containern, Iteratoren und Algorithmen. Dann verwendet man hier und da noch gerne ein "Range"-Konzept. Das definiert jeder ein bisschen anders. Aber ein Ansatz für die Range-Definition ist, dass es sich da um ein leicht kopierbares Objekt mit Referenz-Semantik handelt, welches auf eine Sequenz "zeigt".

So ein ähnliches Konzept habe ich auch mal für die Bildverarbeitung verfolgt. Eine 2D-Raster-Klasse (als Pendent zu std::vector für Bilder), Abstraktionen namens Navigator und Locator (vergleichbar zum Range und Iterator Konzept für 2D) und diverse Algorithmen in Form von Funktionstemplates, die auf den Bildern mit Hilfe von Navigatoren/Locatoren arbeiten.

Ein Algorithmus kann z.B. so aussehen:

template<class Navi1, class Navi2>
void image_copy(Navi1 quelle, Navi2 ziel)
{
  assert( quelle.width() < ziel.width() );
  assert( quelle.height() < ziel.height() );
  auto const q_right = quelle.make_offset(1,0);
  auto const z_right = ziel.make_offset(0,1);
  for (int y=0; y<quelle.height(); ++y) {
    auto q_loc = quelle.make_locator(0,y);
    auto z_loc = ziel.make_locator(0,y);
    for (int x=0; x<quelle.width(); ++x) {
      *z_loc = *q_loc;
      z_loc += z_right;
      q_loc += q_right;
    }
  }
}

Das tolle an dieser Navigator-Abstraktion ist, dass Du Dich nicht auf einen bestimmten Container beschränken musst, der die Pixel speichert. Außerdem kannst Du aus Navigatoren neue erzeugen, die ein transformiertes Bild beschreiben, wobei die Transformation on-the-fly oder auch "lazy" passiert:

template<class PixType>
class raster2d
{
  std::vector<PixType> pixeldata;
  size_t width;
  size_t height;
  :::
  typedef ::: navigator;
  typedef ::: const_navigator;
  :::
  navigator all();
  const_navigator all() const;
  :::
};

int main() {
  raster2d<pixeltyp> dings = ...;
  raster2d<pixeltyp> bums (100,100);
  // Ausschnitt von dings um 90 Grad drehen und nach bums kopieren
  image_copy(dings.all() | crop(20,30,100,100) | rotate90, bums.all());
}

Hier erkennt das geübte Auge, dass ich mich von Boost.Range, den Range-Adaptern und der "Pipe-Syntax" habe inspirieren lassen. Für Navigatoren kam ich bis jetzt mit einer einzigen Implementierung aus:

template<class RandAccessIter>
class linear_navigator
{
public:
  typedef RandAccessIter locator;
  typedef std::iterator_traits<RandAccessIter>::difference_type offset;

  offset make_offset(int x, int y) const { return x*right + y*down;}
  locator make_locator(int x, int y) const { return data+(base+offset(x,y)); }

  size_t width() const {return width_;}
  size_t height() const {return height_;}

  :::

private:
  RandAccessIter data;
  offset base;
  offset right;
  offset down;
  size_t width_;
  size_t height_;
};

Du siehst, man kann in C++ ziemlich coole Sachen bauen. Aber Du kannst das natürlich auch ganz anders stricken. Es ist vielleicht ein bisschen zu viel verlangt am Anfang, wenn Du noch kaum C++ Erfahrung hast.

Aber Du solltest Dich wirklich mit dem Thema mal beschäftigen, weil sich solche Strategien ja durch die Standardbibliothek ziehen.

Cheers!
kk

[Alle Code-Beispiele ohne Gewähr!]

Dobi

@C++ Ausnahmesicherheit: Ja, eine Bildklasse (vielleicht als Wrapper um cv::Mat), die eventuell noch Laden und Speichern kann, hört sich gut an. Ob du das Ganze so sehr kapseln willst, dass man nach außen gar nix von OpenCV merkt, also auch die Algorithmen nicht darauf angewiesen sind, musst du selbst entscheiden. Als Übung sicherlich nicht schlecht, aber praktisch nicht unbedingt nötig. Davon die ganzen Algorithmen als statische Funktionen an die Klasse hängen, würde ich abraten. So eine Gott-Klasse ist dann doch eher ein Anti-Pattern.

Spätestens da, wo deine Software wirklich irgendwas tun soll (und nicht nur eine Bildklasse und Algorithmen hat), wirst du vermutlich noch genug mit Klassen, Ableitungen, Design Patterns und all sowas zu tun bekommen. Was soll sie eigentlich letztendlich tun?

Wenn krümelkackers sehr coole Idee jetzt noch zu seltsam erscheint, ist es für später aber bestimmt mal gut. Das containers-iterators-algorithms-Konzept verstanden zu haben, zahlt sich auf Dauer sehr aus.

Danke für die Tipps.
Werde mir eure Vorschläge mal in Ruhe genau ansehen und mir Gedanken dazu machen.