Lors d'un récent projet sur lequel j'ai travaillé, j'ai dû utiliser beaucoup de fonctions qui ressemblent à ceci:
static bool getGPS(double plane_latitude, double plane_longitude, double plane_altitude,
double plane_roll, double plane_pitch, double plane_heading,
double gimbal_roll, double gimbal_pitch, double gimbal_yaw,
int target_x, int target_y, double zoom,
int image_width_pixels, int image_height_pixels,
double & Target_Latitude, double & Target_Longitude, double & Target_Height);
Je veux donc le refactoriser pour qu'il ressemble à ceci:
static GPSCoordinate getGPS(GPSCoordinate plane, Angle3D planeAngle, Angle3D gimbalAngle,
PixelCoordinate target, ZoomLevel zoom, PixelSize imageSize)
Cela me semble beaucoup plus lisible et sûr que la première méthode. Mais est-il judicieux de créer PixelCoordinate
et de PixelSize
classer? Ou serais-je mieux d'utiliser simplement std::pair<int,int>
pour chacun. Et est-il sensé d'avoir une ZoomLevel
classe, ou devrais-je simplement utiliser un double
?
Mon intuition derrière l'utilisation de classes pour tout est basée sur ces hypothèses:
- S'il y a des classes pour tout, il serait impossible de passer un
ZoomLevel
dans où unWeight
objet était attendu, il serait donc plus difficile de fournir les mauvais arguments à une fonction - De même, certaines opérations illégales entraîneraient des erreurs de compilation, telles que l'ajout d'un
GPSCoordinate
à unZoomLevel
ou un autreGPSCoordinate
- Les opérations juridiques seront faciles à représenter et à sécuriser. c'est-à-dire que la soustraction de deux
GPSCoordinate
s donnerait unGPSDisplacement
Cependant, la plupart du code C ++ que j'ai vu utilise beaucoup de types primitifs, et j'imagine qu'il doit y avoir une bonne raison à cela. Est-ce une bonne idée d'utiliser des objets pour quoi que ce soit, ou a-t-il des inconvénients que je ne connais pas?
Réponses:
Oui définitivement. Les fonctions / méthodes qui prennent trop d'arguments sont une odeur de code et indiquent au moins l'un des éléments suivants:
Si le dernier est le cas (et votre exemple le suggère certainement), il est grand temps de faire une partie de cette "abstraction" de fantaisie dont les enfants cool parlaient oh, il y a seulement quelques décennies. En fait, j'irais plus loin que votre exemple et ferais quelque chose comme:
(Je ne connais pas le GPS, donc ce n'est probablement pas une abstraction correcte; par exemple, je ne comprends pas ce que le zoom a à voir avec une fonction appelée
getGPS
.)Veuillez préférer les cours comme
PixelCoordinate
plusstd::pair<T, T>
, même si les deux ont les mêmes données. Il ajoute une valeur sémantique, plus un peu de sécurité supplémentaire (le compilateur vous empêchera de passer de aScreenCoordinate
à aPixelCoordinate
même si les deux sontpair
s.la source
Avertissement: je préfère le C au C ++
Au lieu de classes, j'utiliserais des structures. Ce point est au mieux pédant, car les structures et les classes sont presque identiques (voir ici pour un graphique simple, ou cette question SO ), mais je pense qu'il y a du mérite dans la différenciation.
Les programmeurs C connaissent les structures en tant que blocs de données qui ont une plus grande signification lorsqu'ils sont regroupés, alors que lorsque je vois des clasas, je suppose qu'il existe un état interne et des méthodes pour manipuler cet état. Une coordonnée GPS n'a pas d'état, juste des données.
D'après votre question, il semble que c'est exactement ce que vous recherchez, un conteneur général, pas une structure avec état. Comme les autres l'ont mentionné, je ne prendrais pas la peine de mettre Zoom dans une classe à part; Personnellement, je déteste les classes conçues pour contenir un type de données (mes professeurs adorent créer
Height
et lesId
classes).Je ne pense pas que ce soit un problème. Si vous réduisez le nombre de paramètres en regroupant les choses évidentes comme vous l'avez fait, les en-têtes devraient être suffisants pour éviter ces problèmes. Si vous êtes vraiment inquiet, séparez les primitives par une structure, qui devrait vous donner des erreurs de compilation pour les erreurs courantes. Il est facile d'échanger deux paramètres l'un à côté de l'autre, mais plus difficile s'ils sont plus éloignés l'un de l'autre.
Bonne utilisation des surcharges de l'opérateur.
Aussi bon usage des surcharges de l'opérateur.
Je pense que vous êtes sur la bonne voie. Une autre chose à considérer est de savoir comment cela s'intègre dans le reste du programme.
la source
L'utilisation de types dédiés a l'avantage qu'il est beaucoup plus difficile de bousiller l'ordre des arguments. La première version de votre exemple de fonction, par exemple, commence par une chaîne de 9 doubles. Lorsque quelqu'un utilise cette fonction, il est très facile de bousiller la commande et de passer les angles du cardan à la place des coordonnées GPS et vice versa. Cela peut conduire à des bogues très étranges et difficiles à tracer. Lorsque vous ne passez que trois arguments avec des types différents à la place, cette erreur peut être détectée par le compilateur.
J'envisagerais en fait d'aller plus loin et d'introduire des types techniquement identiques mais ayant une signification sémantique différente. Par exemple, en ayant une classe
PlaneAngle3d
et une classe distincteGimbalAngle3d
, même si les deux sont une collection de trois doubles. Cela rendrait impossible l'utilisation de l'un comme de l'autre, à moins qu'il ne soit intentionnel.Je recommanderais d'utiliser des typedefs qui ne sont que des alias pour les types primitifs (
typedef double ZoomLevel
) non seulement pour cette raison, mais aussi pour une autre: cela vous permet facilement de changer le type plus tard. Lorsque vous avez un jour l'idée que l'utilisationfloat
pourrait être aussi bonne que,double
mais pourrait être plus efficace en termes de mémoire et de CPU, il vous suffit de changer cela en un seul endroit, et non en des dizaines de fois.la source
Bonnes réponses ici déjà, mais il y a une chose que je voudrais ajouter:
L'utilisation
PixelCoordinate
et laPixelSize
classe rendent les choses très spécifiques. Un généralCoordinate
ou unePoint2D
classe peut probablement mieux répondre à vos besoins. APoint2D
doit être une classe implémentant les opérations ponctuelles / vectorielles les plus courantes. Vous pouvez implémenter une telle classe même de manière générique (Point2D<T>
où T peut êtreint
oudouble
autre chose).Les opérations 2D ponctuelles / vectorielles sont si courantes pour de nombreuses tâches de programmation de géométrie 2D que, selon mon expérience, une telle décision augmentera fortement les chances de réutiliser les opérations 2D existantes. Vous éviterez ainsi la nécessité de les re-mise en œuvre pour chaque
PixelCoordinate
,GPSCoordinate
encore et encore « whatsover » classe -Coordonner.la source
struct PixelCoordinate:Point2D<int>
si vous voulez une sécurité de type appropriéeC'est [plus lisible]. C'est aussi une bonne idée.
Mais est-il judicieux de créer des classes PixelCoordinate et PixelSize?
S'ils représentent des concepts différents, cela a du sens (c'est-à-dire «oui»).
Vous pourriez commencer par faire
typedef std::pair<int,int> PixelCoordinate, PixelSize;
. De cette façon, vous couvrez la sémantique (vous travaillez avec des coordonnées et des tailles de pixels, pas avec des paires std :: dans votre code client) et si vous avez besoin d'étendre, vous remplacez simplement le typedef par une classe et votre code client devrait nécessitent des changements minimes.Vous pouvez également le taper, mais j'utiliserais un
double
jusqu'à ce que j'aie besoin de fonctionnalités associées qui n'existent pas pour un double (par exemple, avoir uneZoomLevel::default
valeur nécessiterait que vous définissiez une classe, mais jusqu'à ce que vous ayez quelque chose comme ça, gardez le double).Ce sont des arguments solides (vous devez toujours vous efforcer de rendre vos interfaces faciles à utiliser correctement et difficiles à utiliser incorrectement).
Il y a des raisons; dans de nombreux cas cependant, ces raisons ne sont pas bonnes. Les performances peuvent être une raison valable pour cela, mais cela signifie que vous devez voir ce type de code comme une exception, pas comme une règle.
C'est généralement une bonne idée de vous assurer que si vos valeurs apparaissent toujours ensemble (et n'ont aucun sens séparément), vous devez les regrouper (pour les mêmes raisons que vous avez mentionnées dans votre message).
Autrement dit, si vous avez des coordonnées qui ont toujours x, y et z, alors il vaut mieux avoir une
coordinate
classe que de transmettre trois paramètres partout.la source