Skip to content

Commit

Permalink
Remove boolean return code in Insert, Rename, Remove methods in KWClass
Browse files Browse the repository at this point in the history 
Contexte:
- les methodes de KWClass et KWClassDomain renvoie false
  - si entite cree ou renomme existe (pour Insert et Rename)
  - si entite detyruite n'existe pas (pour Remove).
- ce comportement a ete source de bug (fuite memoire dans KWClass::CreateClass)
- n'est jamais exploite (sauf quelques rares fois avec bOk = Insert... suivi de assert(bOk)

Amélioration
- passage de ces methode en void
- le controle se fait desormais pas un require
- pas d'impact sur le code existant (sauf quelques rare micro-corrections)

- Methodes concernees
  - KWClassDomain
    - InsertClass
    - RemoveClass
    - DeleteClass
    - RenameClass
    - RenameAttribute
    - InsertDomain
    - RemoveDomain
    - DeleteDomain
    - RenameDomain
  - KWClass
    - InsertAttribute
    - InsertAttributeBefore, InsertAttributeAfter
    - InsertAttributeInBlock
    - RenameAttribute

Tests complets LearningTest passes avec succes
  • Loading branch information
@marcboulle
marcboulle committed Apr 9, 2024
1 parent fd9a5e7 commit e032fb4
Show file tree
Hide file tree
Showing 9 changed files with 340 additions and 444 deletions.
4 changes: 1 addition & 3 deletions src/Learning/DTForest/DTDecisionTreeCreationTask.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2206,7 +2206,6 @@ KWLearningSpec* DTDecisionTreeCreationTask::InitializeRegressionLearningSpec(con
// on transforme la cible continue en cible categorielle, en effectuant au prealable une dicretisation equalFreq
// sur la cible continue

boolean bOk = true;
DTBaseLoader bl;
KWLearningSpec* newLearningSpec = NULL;
KWClass* newClass = NULL;
Expand All @@ -2222,8 +2221,7 @@ KWLearningSpec* DTDecisionTreeCreationTask::InitializeRegressionLearningSpec(con
newTarget->SetName(learningSpec->GetTargetAttributeName() + "_categorical");
newTarget->SetType(KWType::Symbol);
newClass->InsertAttribute(newTarget);
bOk = KWClassDomain::GetCurrentDomain()->InsertClass(newClass);
assert(bOk);
KWClassDomain::GetCurrentDomain()->InsertClass(newClass);
newClass->Compile();
KWClassDomain::GetCurrentDomain()->Compile();
assert(newClass->Check());
Expand Down
3 changes: 1 addition & 2 deletions src/Learning/DTForest/DTDecisionTreeCreationTaskSequential.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1525,8 +1525,7 @@ DTDecisionTreeCreationTaskSequential::InitializeEqualFreqDiscretization(KWTupleT
newTarget->SetName(learningSpec->GetTargetAttributeName() + "_categorical");
newTarget->SetType(KWType::Symbol);
newClass->InsertAttribute(newTarget);
bOk = KWClassDomain::GetCurrentDomain()->InsertClass(newClass);
assert(bOk);
KWClassDomain::GetCurrentDomain()->InsertClass(newClass);
newClass->Compile();
KWClassDomain::GetCurrentDomain()->Compile();
assert(newClass->Check());
Expand Down
290 changes: 142 additions & 148 deletions src/Learning/KWData/KWCYac.cpp
View file

Large diffs are not rendered by default.

12 changes: 3 additions & 9 deletions src/Learning/KWData/KWCYac.yac
View file
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,6 @@ kwclassBegin: kwclassHeader comments
KWClass* kwcClass=$1;
KWAttribute* attribute=$2;
assert(kwcLoadCurrentClass == $1);
boolean bOk;

/* Si attribut non valide: on ne fait rien */
if (attribute == NULL)
Expand Down Expand Up @@ -279,8 +278,7 @@ kwclassBegin: kwclassHeader comments
/* Si OK, d'insertion */
else
{
bOk = kwcClass->InsertAttribute(attribute);
assert(bOk);
kwcClass->InsertAttribute(attribute);
}

$$ = kwcClass;
Expand Down Expand Up @@ -399,7 +397,6 @@ oaAttributeArrayDeclaration :
ObjectArray* oaAttributes = $1;
KWAttribute* attribute=$2;
KWClass* kwcClass=kwcLoadCurrentClass;
boolean bOk;
check(oaAttributes);

/* Si attribut non valide: on ne fait rien */
Expand Down Expand Up @@ -432,9 +429,8 @@ oaAttributeArrayDeclaration :
/* Si OK, d'insertion */
else
{
bOk = kwcClass->InsertAttribute(attribute);
kwcClass->InsertAttribute(attribute);
oaAttributes->Add(attribute);
assert(bOk);
}

$$ = oaAttributes;
Expand All @@ -444,7 +440,6 @@ oaAttributeArrayDeclaration :
ObjectArray* oaAttributes;
KWAttribute* attribute=$1;
KWClass* kwcClass=kwcLoadCurrentClass;
boolean bOk;

/* Creation d'un tableau */
oaAttributes = new ObjectArray;
Expand Down Expand Up @@ -479,9 +474,8 @@ oaAttributeArrayDeclaration :
/* Si OK, d'insertion */
else
{
bOk = kwcClass->InsertAttribute(attribute);
kwcClass->InsertAttribute(attribute);
oaAttributes->Add(attribute);
assert(bOk);
}

$$ = oaAttributes;
Expand Down
169 changes: 69 additions & 100 deletions src/Learning/KWData/KWClass.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,34 +50,28 @@ void KWClass::SetKeyAttributeNameAt(int nIndex, const ALString& sValue)
nFreshness++;
}

boolean KWClass::InsertAttribute(KWAttribute* attribute)
void KWClass::InsertAttribute(KWAttribute* attribute)
{
require(attribute != NULL);
require(CheckName(attribute->GetName(), attribute));
require(attribute->parentClass == NULL);
require(LookupAttribute(attribute->GetName()) == NULL);
require(LookupAttributeBlock(attribute->GetName()) == NULL);

// Test d'existence d'un attribut ou d'un bloc de meme nom
if (odAttributes.Lookup(attribute->GetName()) != NULL or odAttributeBlocks.Lookup(attribute->GetName()) != NULL)
return false;
// Insertion de l'attribut dans le dictionnaire et en fin de liste
else
{
// Ajout dans le dictionnaire des attributs
odAttributes.SetAt(attribute->GetName(), attribute);
attribute->parentClass = this;
// Ajout dans le dictionnaire des attributs
odAttributes.SetAt(attribute->GetName(), attribute);
attribute->parentClass = this;

// Ajout dans la liste
attribute->listPosition = olAttributes.AddTail(attribute);
// Ajout dans la liste
attribute->listPosition = olAttributes.AddTail(attribute);

// Fraicheur
nFreshness++;
// Fraicheur
nFreshness++;

assert(odAttributes.GetCount() == olAttributes.GetCount());
return true;
}
assert(odAttributes.GetCount() == olAttributes.GetCount());
}

boolean KWClass::InsertAttributeBefore(KWAttribute* attribute, KWAttribute* attributeRef)
void KWClass::InsertAttributeBefore(KWAttribute* attribute, KWAttribute* attributeRef)
{
require(attributeRef != NULL);
require(attributeRef == cast(KWAttribute*, odAttributes.Lookup(attributeRef->GetName())));
Expand All @@ -87,29 +81,23 @@ boolean KWClass::InsertAttributeBefore(KWAttribute* attribute, KWAttribute* attr
require(attribute != NULL);
require(CheckName(attribute->GetName(), this));
require(attribute->parentClass == NULL);
require(LookupAttribute(attribute->GetName()) == NULL);
require(LookupAttributeBlock(attribute->GetName()) == NULL);

// Test d'existence d'un attribut ou d'un bloc de meme nom
if (odAttributes.Lookup(attribute->GetName()) != NULL or odAttributeBlocks.Lookup(attribute->GetName()) != NULL)
return false;
// Insertion de l'attribut dans le dictionnaire et dans la liste
else
{
// Ajout dans le dictionnaire
odAttributes.SetAt(attribute->GetName(), attribute);
attribute->parentClass = this;
// Ajout dans le dictionnaire
odAttributes.SetAt(attribute->GetName(), attribute);
attribute->parentClass = this;

// Ajout dans la liste
attribute->listPosition = olAttributes.InsertBefore(attributeRef->listPosition, attribute);
// Ajout dans la liste
attribute->listPosition = olAttributes.InsertBefore(attributeRef->listPosition, attribute);

// Fraicheur
nFreshness++;
// Fraicheur
nFreshness++;

assert(odAttributes.GetCount() == olAttributes.GetCount());
return true;
}
assert(odAttributes.GetCount() == olAttributes.GetCount());
}

boolean KWClass::InsertAttributeAfter(KWAttribute* attribute, KWAttribute* attributeRef)
void KWClass::InsertAttributeAfter(KWAttribute* attribute, KWAttribute* attributeRef)
{
require(attributeRef != NULL);
require(attributeRef == cast(KWAttribute*, odAttributes.Lookup(attributeRef->GetName())));
Expand All @@ -118,72 +106,60 @@ boolean KWClass::InsertAttributeAfter(KWAttribute* attribute, KWAttribute* attri
require(attribute != NULL);
require(CheckName(attribute->GetName(), attribute));
require(attribute->parentClass == NULL);
require(LookupAttribute(attribute->GetName()) == NULL);
require(LookupAttributeBlock(attribute->GetName()) == NULL);

// Test d'existence d'un attribut ou d'un bloc de meme nom
if (odAttributes.Lookup(attribute->GetName()) != NULL or odAttributeBlocks.Lookup(attribute->GetName()) != NULL)
return false;
// Insertion de l'attribut dans le dictionnaire et dans la liste
else
{
// Ajout dans le dictionnaire
odAttributes.SetAt(attribute->GetName(), attribute);
attribute->parentClass = this;
// Ajout dans le dictionnaire
odAttributes.SetAt(attribute->GetName(), attribute);
attribute->parentClass = this;

// Ajout dans la liste
attribute->listPosition = olAttributes.InsertAfter(attributeRef->listPosition, attribute);
// Ajout dans la liste
attribute->listPosition = olAttributes.InsertAfter(attributeRef->listPosition, attribute);

// Fraicheur
nFreshness++;
// Fraicheur
nFreshness++;

assert(odAttributes.GetCount() == olAttributes.GetCount());
return true;
}
assert(odAttributes.GetCount() == olAttributes.GetCount());
}

boolean KWClass::RenameAttribute(KWAttribute* refAttribute, const ALString& sNewName)
void KWClass::RenameAttribute(KWAttribute* refAttribute, const ALString& sNewName)
{
KWAttribute* attribute;
KWDerivationRule* currentDerivationRule;

require(refAttribute != NULL);
require(refAttribute == cast(KWAttribute*, odAttributes.Lookup(refAttribute->GetName())));
require(refAttribute->parentClass == this);
require(LookupAttribute(sNewName) == NULL);
require(CheckName(sNewName, refAttribute));

// Test d'existence d'un attribut ou d'un bloc de meme nom
if (odAttributes.Lookup(sNewName) != NULL or odAttributeBlocks.Lookup(sNewName) != NULL)
return false;
// Renommage par manipulation dans le dictionnaire
else
// Propagation du renommage a toutes les regles de derivation
// des classes du domaine referencant l'attribut
attribute = GetHeadAttribute();
currentDerivationRule = NULL;
while (attribute != NULL)
{
// Propagation du renommage a toutes les regles de derivation
// des classes du domaine referencant l'attribut
attribute = GetHeadAttribute();
currentDerivationRule = NULL;
while (attribute != NULL)
// Detection de changement de regle de derivation (notamment pour les blocs)
if (attribute->GetAnyDerivationRule() != currentDerivationRule)
{
// Detection de changement de regle de derivation (notamment pour les blocs)
if (attribute->GetAnyDerivationRule() != currentDerivationRule)
{
currentDerivationRule = attribute->GetAnyDerivationRule();

// Renommage dans les regles de derivation (et au plus une seule fois par bloc)
if (currentDerivationRule != NULL)
currentDerivationRule->RenameAttribute(this, refAttribute, sNewName);
}
currentDerivationRule = attribute->GetAnyDerivationRule();

// Attribut suivant
GetNextAttribute(attribute);
// Renommage dans les regles de derivation (et au plus une seule fois par bloc)
if (currentDerivationRule != NULL)
currentDerivationRule->RenameAttribute(this, refAttribute, sNewName);
}

// Renommage de l'attribut dans la classe
odAttributes.RemoveKey(refAttribute->GetName());
refAttribute->usName.SetValue(sNewName);
odAttributes.SetAt(refAttribute->GetName(), refAttribute);
assert(odAttributes.GetCount() == olAttributes.GetCount());
nFreshness++;
return true;
// Attribut suivant
GetNextAttribute(attribute);
}

// Renommage de l'attribut dans la classe
odAttributes.RemoveKey(refAttribute->GetName());
refAttribute->usName.SetValue(sNewName);
odAttributes.SetAt(refAttribute->GetName(), refAttribute);
assert(odAttributes.GetCount() == olAttributes.GetCount());
nFreshness++;
}

void KWClass::UnsafeRenameAttribute(KWAttribute* refAttribute, const ALString& sNewName)
Expand Down Expand Up @@ -820,7 +796,7 @@ KWAttributeBlock* KWClass::CreateAttributeBlock(const ALString& sBlockName, KWAt
return attributeBlock;
}

boolean KWClass::InsertAttributeInBlock(KWAttribute* attribute, KWAttributeBlock* attributeBlockRef)
void KWClass::InsertAttributeInBlock(KWAttribute* attribute, KWAttributeBlock* attributeBlockRef)
{
require(attribute != NULL);
require(CheckName(attribute->GetName(), attribute));
Expand All @@ -833,31 +809,24 @@ boolean KWClass::InsertAttributeInBlock(KWAttribute* attribute, KWAttributeBlock
attributeBlockRef->GetFirstAttribute());
require(LookupAttribute(attributeBlockRef->GetLastAttribute()->GetName()) ==
attributeBlockRef->GetLastAttribute());
require(LookupAttribute(attribute->GetName()) == NULL);
require(LookupAttributeBlock(attribute->GetName()) == NULL);

// Test d'existence d'un attribut ou d'un bloc de meme nom
if (odAttributes.Lookup(attribute->GetName()) != NULL or odAttributeBlocks.Lookup(attribute->GetName()) != NULL)
return false;
// Insertion de l'attribut dans le dictionnaire et en fin de liste
else
{
// Ajout dans le dictionnaire des attributs
odAttributes.SetAt(attribute->GetName(), attribute);
attribute->parentClass = this;
// Ajout dans le dictionnaire des attributs
odAttributes.SetAt(attribute->GetName(), attribute);
attribute->parentClass = this;

// Ajout dans la liste, apres le dernier attribut du bloc
attribute->listPosition =
olAttributes.InsertAfter(attributeBlockRef->lastAttribute->listPosition, attribute);
// Ajout dans la liste, apres le dernier attribut du bloc
attribute->listPosition = olAttributes.InsertAfter(attributeBlockRef->lastAttribute->listPosition, attribute);

// Mise a jour des informations sur le block
attribute->attributeBlock = attributeBlockRef;
attributeBlockRef->lastAttribute = attribute;
// Mise a jour des informations sur le block
attribute->attributeBlock = attributeBlockRef;
attributeBlockRef->lastAttribute = attribute;

// Fraicheur
nFreshness++;
// Fraicheur
nFreshness++;

assert(odAttributes.GetCount() == olAttributes.GetCount());
return true;
}
assert(odAttributes.GetCount() == olAttributes.GetCount());
}

KWAttributeBlock* KWClass::LookupAttributeBlock(const ALString& sBlockName) const
Expand Down
19 changes: 9 additions & 10 deletions src/Learning/KWData/KWClass.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -122,23 +122,22 @@ class KWClass : public Object
KWAttribute* LookupAttribute(const ALString& sAttributeName) const;

// Ajout d'un attribut en fin de liste, et par rapport a un autre attribut
// Renvoie true si OK, false sinon (attribut ou bloc existant)
boolean InsertAttribute(KWAttribute* attribute);
// Le nom de l'attribut ne doit pas deja exister
void InsertAttribute(KWAttribute* attribute);

// Insertion avant ou apres un attribut existant
// Erreur de programmation si attribut de reference inexistant,
// ou au milieu d'un bloc (on peut etre au debut d'un bloc pour
// le InsertBefore ou a la fin d'un bloc pour le InsertAfter)
// Renvoie true si OK, false sinon (attribut ou bloc existant)
boolean InsertAttributeBefore(KWAttribute* attribute, KWAttribute* attributeRef);
boolean InsertAttributeAfter(KWAttribute* attribute, KWAttribute* attributeRef);
// Le nom de l'attribut ne doit pas deja exister
void InsertAttributeBefore(KWAttribute* attribute, KWAttribute* attributeRef);
void InsertAttributeAfter(KWAttribute* attribute, KWAttribute* attributeRef);

// Renommage d'un attribut
// Retourne true si OK. Sans effet si le nom cible existe deja
// parmi les attribut ou les blocs.
// Le nom cible ne doit pas exister parmi les attribut ou les blocs.
// Propagation a toutes les references a cet attribut dans les regles
// de derivations des attributs de la classe
boolean RenameAttribute(KWAttribute* refAttribute, const ALString& sNewName);
void RenameAttribute(KWAttribute* refAttribute, const ALString& sNewName);

// Renommage d'un attribut sans se soucier des utilisation dans les regles
// Le nom doit etre inexistant dans la classe
Expand Down Expand Up @@ -248,8 +247,8 @@ class KWClass : public Object
KWAttribute* lastAttribute);

// Ajout d'un attribut en fin d'un bloc existant
// Renvoie true si OK, false sinon (attribut ou bloc existant)
boolean InsertAttributeInBlock(KWAttribute* attribute, KWAttributeBlock* attributeBlockRef);
// Le nom de l'attribut ne doit pas deja exister
void InsertAttributeInBlock(KWAttribute* attribute, KWAttributeBlock* attributeBlockRef);

// Recherche d'un bloc par son nom
// Retourne NUL si bloc inexistant
Expand Down
Loading

0 comments on commit e032fb4

Please sign in to comment.