diff --git a/source/worksheet/worksheet.cpp b/source/worksheet/worksheet.cpp index e1d83379..5d907e77 100644 --- a/source/worksheet/worksheet.cpp +++ b/source/worksheet/worksheet.cpp @@ -242,25 +242,31 @@ std::string worksheet::title() const void worksheet::title(const std::string &title) { - if (title.length() > 31) + // do no work if we don't need to + if (d_->title_ == title) + { + return; + } + // excel limits worksheet titles to 31 characters + if (title.empty() || title.length() > 31) { throw invalid_sheet_title(title); } - + // invalid characters in a worksheet name if (title.find_first_of("*:/\\?[]") != std::string::npos) { throw invalid_sheet_title(title); } - - auto same_title = std::find_if(workbook().begin(), workbook().end(), - [&](worksheet ws) { return ws.title() == title; }); - - if (same_title != workbook().end() && *same_title != *this) + // try and insert the new name into the worksheets map + // if the insert fails, we have a duplicate sheet name + auto insert_result = workbook().d_->sheet_title_rel_id_map_.insert( + std::make_pair(title, workbook().d_->sheet_title_rel_id_map_[d_->title_])); + if (!insert_result.second) // insert failed, duplication detected { throw invalid_sheet_title(title); } - - workbook().d_->sheet_title_rel_id_map_[title] = workbook().d_->sheet_title_rel_id_map_[d_->title_]; + // if the insert succeeded (i.e. wasn't a duplicate sheet name) + // update the worksheet title and remove the old relation workbook().d_->sheet_title_rel_id_map_.erase(d_->title_); d_->title_ = title; diff --git a/tests/worksheet/worksheet_test_suite.hpp b/tests/worksheet/worksheet_test_suite.hpp index d3ddb649..5b7f6736 100644 --- a/tests/worksheet/worksheet_test_suite.hpp +++ b/tests/worksheet/worksheet_test_suite.hpp @@ -25,10 +25,10 @@ #include -#include #include #include #include +#include class worksheet_test_suite : public test_suite { @@ -101,6 +101,7 @@ public: register_test(test_view_properties_serialization); register_test(test_clear_cell); register_test(test_clear_row); + register_test(test_set_title); } void test_new_worksheet() @@ -376,7 +377,6 @@ public: xlnt_assert(!merged.contains("O1")); } - void test_merged_cell_ranges() { xlnt::workbook wb; @@ -391,7 +391,7 @@ public: ws.cell("A1").value(1); ws.cell("D4").value(16); ws.merge_cells("A1:D4"); - std::vector expected = { xlnt::range_reference("A1:D4") }; + std::vector expected = {xlnt::range_reference("A1:D4")}; xlnt_assert_equals(ws.merged_ranges(), expected); xlnt_assert(!ws.cell("D4").has_value()); } @@ -825,7 +825,7 @@ public: xlnt::header_footer hf; using hf_loc = xlnt::header_footer::location; - for (auto location : { hf_loc::left, hf_loc::center, hf_loc::right }) + for (auto location : {hf_loc::left, hf_loc::center, hf_loc::right}) { xlnt_assert(!hf.has_header(location)); xlnt_assert(!hf.has_odd_even_header(location)); @@ -850,7 +850,7 @@ public: xlnt::header_footer hf; using hf_loc = xlnt::header_footer::location; - for (auto location : { hf_loc::left, hf_loc::center, hf_loc::right }) + for (auto location : {hf_loc::left, hf_loc::center, hf_loc::right}) { xlnt_assert(!hf.has_footer(location)); xlnt_assert(!hf.has_odd_even_footer(location)); @@ -1186,11 +1186,11 @@ public: xlnt_assert_equals(ws.highest_row(), last_row); wb.save("temp.xlsx"); - + xlnt::workbook wb2; wb2.load("temp.xlsx"); auto ws2 = wb2.active_sheet(); - + xlnt_assert_equals(ws2.calculate_dimension().height(), height); xlnt_assert(!ws2.has_cell(xlnt::cell_reference(1, last_row))); } @@ -1212,12 +1212,42 @@ public: xlnt_assert_equals(ws.highest_row(), last_row - 1); wb.save("temp.xlsx"); - + xlnt::workbook wb2; wb2.load("temp.xlsx"); auto ws2 = wb2.active_sheet(); - + xlnt_assert_equals(ws2.calculate_dimension().height(), height - 1); xlnt_assert(!ws2.has_cell(xlnt::cell_reference(1, last_row))); } + + void test_set_title() + { + xlnt::workbook wb; + auto ws1 = wb.active_sheet(); + // empty titles are invalid + xlnt_assert_throws(ws1.title(""), xlnt::invalid_sheet_title); + // titles longer than 31 chars are invalid + std::string test_long_title(32, 'a'); + xlnt_assert(test_long_title.size() > 31); + xlnt_assert_throws(ws1.title(test_long_title), xlnt::invalid_sheet_title); + // titles containing any of the following characters are invalid + std::string invalid_chars = "*:/\\?[]"; + for (char &c : invalid_chars) + { + std::string invalid_char = std::string("Sheet") + c; + xlnt_assert_throws(ws1.title(invalid_char), xlnt::invalid_sheet_title); + } + // duplicate names are invalid + auto ws2 = wb.create_sheet(); + xlnt_assert_throws(ws2.title(ws1.title()), xlnt::invalid_sheet_title); + xlnt_assert_throws(ws1.title(ws2.title()), xlnt::invalid_sheet_title); + // naming as self is valid and is ignored + auto ws1_title = ws1.title(); + auto ws2_title = ws2.title(); + xlnt_assert_throws_nothing(ws1.title(ws1.title())); + xlnt_assert_throws_nothing(ws2.title(ws2.title())); + xlnt_assert(ws1_title == ws1.title()); + xlnt_assert(ws2_title == ws2.title()); + } };