From 11f6c2f993c98c2a3bce99bcdd560a2c27eb84f0 Mon Sep 17 00:00:00 2001
From: Ray Speth <speth@mit.edu>
Date: Sat, 10 Sep 2016 21:43:44 -0400
Subject: [PATCH] Consolidate duplicate species checks in Phase::addSpecies

---
 interfaces/cython/cantera/ctml_writer.py      |  4 ---
 .../cython/cantera/test/test_convert.py       |  2 +-
 interfaces/cython/cantera/test/test_thermo.py |  5 ++++
 src/thermo/Phase.cpp                          |  5 ++++
 src/thermo/ThermoFactory.cpp                  | 28 ++-----------------
 5 files changed, 14 insertions(+), 30 deletions(-)

diff --git a/interfaces/cython/cantera/ctml_writer.py b/interfaces/cython/cantera/ctml_writer.py
index 843f7197b..08b13395b 100644
--- a/interfaces/cython/cantera/ctml_writer.py
+++ b/interfaces/cython/cantera/ctml_writer.py
@@ -585,8 +585,6 @@ class species(object):
         global _enames
         _species.append(self)
         global _speciesnames
-        if name in _speciesnames:
-            raise CTI_Error('species '+name+' multiply defined.')
         _speciesnames.append(name)
         for e in self._atoms.keys():
             _enames[e] = 1
@@ -1783,8 +1781,6 @@ class phase(object):
                 self._sp.append(('', spnames))
 
             for s in spnames.split():
-                if s != 'all' and s in self._spmap:
-                    raise CTI_Error('Multiply-declared species '+s+' in phase '+self._name)
                 self._spmap[s] = self._dim
 
         self._rxns = reactions
diff --git a/interfaces/cython/cantera/test/test_convert.py b/interfaces/cython/cantera/test/test_convert.py
index 4817b53c1..350bf25cd 100644
--- a/interfaces/cython/cantera/test/test_convert.py
+++ b/interfaces/cython/cantera/test/test_convert.py
@@ -427,7 +427,7 @@ class CtmlConverterTest(utilities.CanteraTest):
         except RuntimeError as e:
             err = e
 
-        self.assertIn('Multiply-declared species', err.args[0])
+        self.assertIn('already contains', err.args[0])
 
     def test_noninteger_atomicity(self):
         gas = ct.Solution('../data/noninteger-atomicity.cti')
diff --git a/interfaces/cython/cantera/test/test_thermo.py b/interfaces/cython/cantera/test/test_thermo.py
index cf8d17555..5e048252e 100644
--- a/interfaces/cython/cantera/test/test_thermo.py
+++ b/interfaces/cython/cantera/test/test_thermo.py
@@ -621,6 +621,11 @@ class TestThermoPhase(utilities.CanteraTest):
         gc.collect()
         self.phase.add_species(ref.species('CH2O'))
 
+    def test_add_species_duplicate(self):
+        species = self.phase.species('H2O2')
+        with self.assertRaises(RuntimeError):
+            self.phase.add_species(species)
+
 
 class TestThermo(utilities.CanteraTest):
     def setUp(self):
diff --git a/src/thermo/Phase.cpp b/src/thermo/Phase.cpp
index 4981835d6..f5919a568 100644
--- a/src/thermo/Phase.cpp
+++ b/src/thermo/Phase.cpp
@@ -757,6 +757,11 @@ size_t Phase::addElement(const std::string& symbol, doublereal weight,
 }
 
 bool Phase::addSpecies(shared_ptr<Species> spec) {
+    if (m_species.find(spec->name) != m_species.end()) {
+        throw CanteraError("Phase::addSpecies",
+            "Phase '{}' already contains a species named '{}'.",
+            m_name, spec->name);
+    }
     m_species[spec->name] = spec;
     vector_fp comp(nElements());
     for (const auto& elem : spec->composition) {
diff --git a/src/thermo/ThermoFactory.cpp b/src/thermo/ThermoFactory.cpp
index babcad801..0f3e43c7c 100644
--- a/src/thermo/ThermoFactory.cpp
+++ b/src/thermo/ThermoFactory.cpp
@@ -190,16 +190,7 @@ static void formSpeciesXMLNodeList(std::vector<XML_Node*> &spDataNodeList,
             spnames.resize(nsp);
             for (size_t nn = 0; nn < nsp; nn++) {
                 string stemp = (*allsp[nn])["name"];
-                bool skip = false;
-                if (declared[stemp]) {
-                    if (sprule[jsp] >= 10) {
-                        skip = true;
-                    } else {
-                        throw CanteraError("ThermoFactory::formSpeciesXMLNodeList()",
-                                           "duplicate species: \"" + stemp + "\"");
-                    }
-                }
-                if (!skip) {
+                if (!declared[stemp] || sprule[jsp] < 10) {
                     declared[stemp] = true;
                     spNamesList.push_back(stemp);
                     spDataNodeList.push_back(allsp[nn]);
@@ -212,11 +203,7 @@ static void formSpeciesXMLNodeList(std::vector<XML_Node*> &spDataNodeList,
             spnames.resize(nsp);
             for (size_t nn = 0; nn < nsp; nn++) {
                 string stemp = (*allsp[nn])["name"];
-                bool skip = false;
-                if (declared[stemp]) {
-                    skip = true;
-                }
-                if (!skip) {
+                if (!declared[stemp]) {
                     declared[stemp] = true;
                     spNamesList.push_back(stemp);
                     spDataNodeList.push_back(allsp[nn]);
@@ -231,16 +218,7 @@ static void formSpeciesXMLNodeList(std::vector<XML_Node*> &spDataNodeList,
             }
             for (size_t k = 0; k < nsp; k++) {
                 string stemp = spnames[k];
-                bool skip = false;
-                if (declared[stemp]) {
-                    if (sprule[jsp] >= 10) {
-                        skip = true;
-                    } else {
-                        throw CanteraError("ThermoFactory::formSpeciesXMLNodeList()",
-                                           "duplicate species: \"" + stemp + "\"");
-                    }
-                }
-                if (!skip) {
+                if (!declared[stemp] || sprule[jsp] < 10) {
                     declared[stemp] = true;
                     // Find the species in the database by name.
                     auto iter = speciesNodes.find(stemp);