search

olivierdalang / CadInput

CadInput QGIS plugin
10 stars 1 forks source link

do not draw undefined previous points (and adapt constraints) and right clicks ends construction #16

Closed 3nids closed 10 years ago

3nids commented 10 years ago

Salut, Voilà, j'ai mis à jour. J'ai mis les fonctions d'intersection dans une nouvelle classe pour que ça soit plus digeste dans CadEventFilter.

3nids commented 10 years ago

J'ai ajouté une nouvelle classe CadConstraintCapabilities qui gère ce qui est possible de faire avec le stock de points. Je l'ai fait en dehors de CadPointList car elle est utilisée à vide dans CadInputWidget (avant la création de CadPointList dans CadEventFilter).

3nids commented 10 years ago

Je me rend compte que j'ai rajouté pas mal de classes (certes petites) à ce que tu avais à l'origine. C'est dans mes habitudes de répartir au max le code, mais apparemment tu préfères regrouper.

Si cela te semble plus clair, je peux remettre les classes CadPointList, CadConstraintCapabilities et CadIntersection dans CadEventFilter.

3nids commented 10 years ago

Le dernier problème (connu) a été corrigé. Redis moi si tu veux que je fasse un rebase.

olivierdalang commented 10 years ago

Hello, non laisse comme ça, je vais regarder ça ce soir et je te tiens au jus !

2014-02-05 Denis Rouzaud notifications@github.com:

Le dernier problème (connu) a été corrigé. Redis moi si tu veux que je fasse un rebase.

— Reply to this email directly or view it on GitHubhttps://github.com/olivierdalang/CadInput/pull/16#issuecomment-34163933 .

3nids commented 10 years ago

ok, parfait!

olivierdalang commented 10 years ago

OK je suis en train de tester, ça a l'air de bien marcher !

Pour la classe "CadConstraintCapabilities", je suis en effet plutôt pour laisser ces méthodes dans CadInputWidget. Il me semble qu'il s'agit là vraiment de méthodes liées au CadInputWidget (en gros, enabler ou non les Widgets).

Je pense qu'on peut enlever les question de "hasChanged" : c'est une optimisation qui n'est pas utile puisque les seuls moments ou CadConstraintCapabilities.update() est appelé est lorsqu'un point est ajouté, et de toutes façons, la methode CadInputWidget.enableConstraints est quasi gratuite (faire setEnabled(True) a un Widget qui est déjà enabled n'a pas d'effet).

Sans les hasChanged, il reste la methode update, qu'on peut alors directement implementer dans CadInputWidget...

Je fais les modifs, tu me diras si j'ai raté quelque chose !

olivierdalang commented 10 years ago

Ok c'est fait... C'est un peu plus simple je trouve sans cette classe, parce que les conditions type if len(self.cadPointList)>1: sont assez lisibles dans le code.

Et si à l'avenir on a des capabilities plus complexes (et pas seulement basées sur le nombre de points), on pourra toujours revenir a une solution une classe dédiée. Mais je pense que c'est mieux de garder le code au plus simple à ce stade ! ( => http://en.wikipedia.org/wiki/YAGNI )

3nids commented 10 years ago

Hello!

OK, je comprend. Le truc c'est que update est appelé dès qu'un point est ajouté (à chaque clic) mais je savais pas pas que setEnabled n'avait pas d'effet. Donc, +1.

Je regarde tes autres commentaires.

Le 5 février 2014 19:06, olivierdalang notifications@github.com a écrit :

Ok c'est fait... C'est un peu plus simple je trouve sans cette classe, parce que les conditions type if len(self.cadPointList)>1: sont assez lisibles dans le code.

Et si à l'avenir on a des capabilities plus complexes (et pas seulement basées sur le nombre de points), on pourra toujours revenir a une solution une classe dédiée. Mais je pense que c'est mieux de garder le code au plus simple à ce stade ! ( => http://en.wikipedia.org/wiki/YAGNI )

Reply to this email directly or view it on GitHubhttps://github.com/olivierdalang/CadInput/pull/16#issuecomment-34218587 .

3nids commented 10 years ago

OK, merci pour tes corrections! Je peux pas tester depuis chez moi, mais je suis d'accord avec l'idée! A+ Denis

Le 5 février 2014 19:44, Denis Rouzaud denis.rouzaud@gmail.com a écrit :

Hello!

OK, je comprend. Le truc c'est que update est appelé dès qu'un point est ajouté (à chaque clic) mais je savais pas pas que setEnabled n'avait pas d'effet. Donc, +1.

Je regarde tes autres commentaires.

Le 5 février 2014 19:06, olivierdalang notifications@github.com a écrit :

Ok c'est fait... C'est un peu plus simple je trouve sans cette classe,

parce que les conditions type if len(self.cadPointList)>1: sont assez lisibles dans le code.

Et si à l'avenir on a des capabilities plus complexes (et pas seulement basées sur le nombre de points), on pourra toujours revenir a une solution une classe dédiée. Mais je pense que c'est mieux de garder le code au plus simple à ce stade ! ( => http://en.wikipedia.org/wiki/YAGNI )

Reply to this email directly or view it on GitHubhttps://github.com/olivierdalang/CadInput/pull/16#issuecomment-34218587 .