obiwan_kennedyI would like to show the evolution of one function in rolisteam code.
This function is called when the user (Game Master) closes a map or image.
The function must close the map or image and send a network order to each player to close the image or plan.
At the beginning, the function looked like that.
Some code line and comments were written in French. No coding rules were respected.
Low-level code for networking. The function was very long.
// On recupere la fenetre active (qui est forcement de type CarteFenetre ou Image, sans quoi l'action
// ne serait pas dispo dans le menu Fichier)
QWidget *active = workspace->activeWindow();
// Ne devrait jamais arriver
if (!active)
{
qWarning("Close map action called when no widget is active in the workspace (fermerPlanOuImage - MainWindow.h)");
return;
}
// On verifie pour le principe qu'il s'agit bien d'une CarteFenetre ou d'une Image
if (active->objectName() != "CarteFenetre" && active->objectName() != "Image")
{
qWarning("not expected type of windows (fermerPlanOuImage - MainWindow.h)");
return;
}
// Creation de la boite d'alerte
QMessageBox msgBox(this);
msgBox.addButton(QMessageBox::Yes);
msgBox.addButton(QMessageBox::Cancel);
msgBox.setIcon(QMessageBox::Information);
msgBox.move(QPoint(width()/2, height()/2) + QPoint(-100, -50));
// On supprime l'icone de la barre de titre
Qt::WindowFlags flags = msgBox.windowFlags();
msgBox.setWindowFlags(flags ^ Qt::WindowSystemMenuHint);
// M.a.j du titre et du message
if (active->objectName() == "CarteFenetre")
{
msgBox.setWindowTitle(tr("Close Map"));
msgBox.setText(tr("Do you want to close this map?\nIt will be closed for everybody"));
}
else
{
msgBox.setWindowTitle(tr("Close Picture"));
msgBox.setText(tr("Do you want to close this picture?\nIt will be closed for everybody"));
}
msgBox.exec();
// Si l'utilisateur n'a pas clique sur "Fermer", on quitte
if (msgBox.result() != QMessageBox::YesRole)
return;
// Emission de la demande de fermeture de la carte
if (active->objectName() == "CarteFenetre")
{
// Recuperation de l'identifiant de la carte
QString idCarte = ((CarteFenetre *)active)->carte()->identifiantCarte();
// Taille des donnees
quint32 tailleCorps =
// Taille de l'identifiant de la carte
sizeof(quint8) + idCarte.size()*sizeof(QChar);
// Buffer d'emission
char *donnees = new char[tailleCorps + sizeof(enteteMessage)];
// Creation de l'entete du message
enteteMessage *uneEntete;
uneEntete = (enteteMessage *) donnees;
uneEntete->categorie = plan;
uneEntete->action = fermerPlan;
uneEntete->tailleDonnees = tailleCorps;
// Creation du corps du message
int p = sizeof(enteteMessage);
// Ajout de l'identifiant de la carte
quint8 tailleIdCarte = idCarte.size();
memcpy(&(donnees[p]), &tailleIdCarte, sizeof(quint8));
p+=sizeof(quint8);
memcpy(&(donnees[p]), idCarte.data(), tailleIdCarte*sizeof(QChar));
p+=tailleIdCarte*sizeof(QChar);
// Emission de la demande de fermeture de la carte au serveur ou a l'ensemble des clients
emettre(donnees, tailleCorps + sizeof(enteteMessage));
// Liberation du buffer d'emission
delete[] donnees;
// Suppression de la CarteFenetre et de l'action associee sur l'ordinateur local
((CarteFenetre *)active)->~CarteFenetre();
}
// Emission de la demande de fermeture de l'image
else
{
// Recuperation de l'identifiant de la carte
QString idImage = ((Image *)active)->identifiantImage();
// Taille des donnees
quint32 tailleCorps =
// Taille de l'identifiant de la carte
sizeof(quint8) + idImage.size()*sizeof(QChar);
// Buffer d'emission
char *donnees = new char[tailleCorps + sizeof(enteteMessage)];
// Creation de l'entete du message
enteteMessage *uneEntete;
uneEntete = (enteteMessage *) donnees;
uneEntete->categorie = image;
uneEntete->action = fermerImage;
uneEntete->tailleDonnees = tailleCorps;
// Creation du corps du message
int p = sizeof(enteteMessage);
// Ajout de l'identifiant de la carte
quint8 tailleIdImage = idImage.size();
memcpy(&(donnees[p]), &tailleIdImage, sizeof(quint8));
p+=sizeof(quint8);
memcpy(&(donnees[p]), idImage.data(), tailleIdImage*sizeof(QChar));
p+=tailleIdImage*sizeof(QChar);
// Emission de la demande de fermeture de l'image au serveur ou a l'ensemble des clients
emettre(donnees, tailleCorps + sizeof(enteteMessage));
// Liberation du buffer d'emission
delete[] donnees;
// Suppression de l'Image et de l'action associee sur l'ordinateur local
((Image *)active)->~Image();
}A big step forward, the networking code has been reworked.
So the readability is improved. It is also possible to see the beginning of polymorphism. MediaContener is used but not everywhere.
QMdiSubWindow* subactive = m_mdiArea->currentSubWindow();
QWidget* active = subactive;
MapFrame* bipMapWindow = NULL;
if (NULL!=active)
{
QAction* action=NULL;
Image* imageFenetre = dynamic_cast(active);
QString mapImageId;
QString mapImageTitle;
mapImageTitle = active->windowTitle();
bool image=false;
//it is image
if(NULL!=imageFenetre)
{
m_pictureList.removeOne(imageFenetre);
mapImageId = imageFenetre->getMediaId();
image = true;
action = imageFenetre->getAction();
}
else//it is a map
{
bipMapWindow= dynamic_cast(active);
if(NULL!=bipMapWindow)
{
mapImageId = bipMapWindow->getMediaId();
action = bipMapWindow->getAction();
}
else// it is undefined
{
return;
}
}
QMessageBox msgBox(this);
msgBox.setStandardButtons(QMessageBox::Yes | QMessageBox::Cancel );
msgBox.setDefaultButton(QMessageBox::Cancel);
msgBox.setIcon(QMessageBox::Information);
msgBox.move(QPoint(width()/2, height()/2) + QPoint(-100, -50));
Qt::WindowFlags flags = msgBox.windowFlags();
msgBox.setWindowFlags(flags ^ Qt::WindowSystemMenuHint);
if (!image)
{
msgBox.setWindowTitle(tr("Close Map"));
}
else
{
msgBox.setWindowTitle(tr("Close Picture"));
}
msgBox.setText(tr("Do you want to close %1 %2?\nIt will be closed for everybody").arg(mapImageTitle).arg(image?tr(""):tr("(Map)")));
msgBox.exec();
if (msgBox.result() != QMessageBox::Yes)
return;
if (!image)
{
NetworkMessageWriter msg(NetMsg::MapCategory,NetMsg::CloseMap);
msg.string8(mapImageId);
msg.sendAll();
m_mapWindowMap.remove(mapImageId);
m_playersListWidget->model()->changeMap(NULL);
m_toolBar->changeMap(NULL);
}
else
{
NetworkMessageWriter msg(NetMsg::PictureCategory,NetMsg::DelPictureAction);
msg.string8(mapImageId);
msg.sendAll();
}
MediaContainer* mediaContener = dynamic_cast(subactive);
if(NULL!=mediaContener)
{
CleverURI* cluri = mediaContener->getCleverUri();
cluri->setDisplayed(false);
if(NULL!=m_sessionManager)
{
m_sessionManager->updateCleverUri(cluri);
}
}
delete action;
delete subactive;
}Version Intermédiaire
Then, we implements a solution to describe map, vmap (v1.8) or image as mediacontener.
They share a large part of their code, so it becomes really easy to do it.
QMdiSubWindow* subactive = m_mdiArea->currentSubWindow();
MediaContainer* container = dynamic_cast(subactive);
if(NULL != container)
{
CleverURI::ContentType type = container->getContentType();
if(CleverURI::VMAP == type)
{
removeVMapFromId(container->getMediaId());
}
else if(CleverURI::MAP == type)
{
removeMapFromId(container->getMediaId());
}
else if(CleverURI::PICTURE == type )
{
removePictureFromId(container->getMediaId());
}
}Actuel
sagarhani
