Skip to content

Commit

Permalink
Refresh help_impl.cpp using ranged-for loops and avoiding raw pointers
Browse files Browse the repository at this point in the history 
The old copy-assignment code would have appended the topics and sections
without clearing those lists, while the new code does clear them. However, the
copy-assignment is only called after calling clear(), so this doesn't affect
the behavior.
  • Loading branch information
@stevecotton
stevecotton committed Aug 6, 2019
1 parent 32a9acf commit 06f0776
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 76 deletions.
61 changes: 14 additions & 47 deletions src/help/help_impl.cpp
View file
Expand Up @@ -181,9 +181,8 @@ void parse_config_internal(const config *help_cfg, const config *section_cfg,
}

generate_sections(help_cfg, (*section_cfg)["sections_generator"], sec, level);
//TODO: harmonize topics/sections sorting
if ((*section_cfg)["sort_sections"] == "yes") {
std::sort(sec.sections.begin(),sec.sections.end(), section_less());
sec.sections.sort(section_less());
}

bool sort_topics = false;
Expand Down Expand Up @@ -1043,18 +1042,16 @@ std::string generate_contents_links(const section &sec, const std::vector<topic>
{
std::stringstream res;

section_list::const_iterator s;
for (s = sec.sections.begin(); s != sec.sections.end(); ++s) {
if (is_visible_id((*s)->id)) {
std::string link = make_link((*s)->title, ".."+(*s)->id);
for (auto &s : sec.sections) {
if (is_visible_id(s.id)) {
std::string link = make_link(s.title, ".."+s.id);
res << font::unicode_bullet << " " << link << "\n";
}
}

std::vector<topic>::const_iterator t;
for (t = topics.begin(); t != topics.end(); ++t) {
if (is_visible_id(t->id)) {
std::string link = make_link(t->title, t->id);
for (auto &t : topics) {
if (is_visible_id(t.id)) {
std::string link = make_link(t.title, t.id);
res << font::unicode_bullet << " " << link << "\n";
}
}
Expand All @@ -1071,34 +1068,6 @@ bool topic::operator<(const topic &t) const
return id < t.id;
}

section::~section()
{
std::for_each(sections.begin(), sections.end(), delete_section());
}

section::section(const section &sec) :
title(sec.title),
id(sec.id),
topics(sec.topics),
sections(),
level(sec.level)
{
std::transform(sec.sections.begin(), sec.sections.end(),
std::back_inserter(sections), create_section());
}

section& section::operator=(const section &sec)
{
title = sec.title;
id = sec.id;
level = sec.level;
topics.insert(topics.end(), sec.topics.begin(), sec.topics.end());
std::transform(sec.sections.begin(), sec.sections.end(),
std::back_inserter(sections), create_section());
return *this;
}


bool section::operator==(const section &sec) const
{
return sec.id == id;
Expand All @@ -1111,13 +1080,12 @@ bool section::operator<(const section &sec) const

void section::add_section(const section &s)
{
sections.push_back(new section(s));
sections.emplace_back(s);
}

void section::clear()
{
topics.clear();
std::for_each(sections.begin(), sections.end(), delete_section());
sections.clear();
}

Expand All @@ -1130,9 +1098,8 @@ const topic *find_topic(const section &sec, const std::string &id)
if (tit != sec.topics.end()) {
return &(*tit);
}
section_list::const_iterator sit;
for (sit = sec.sections.begin(); sit != sec.sections.end(); ++sit) {
const topic *t = find_topic(*(*sit), id);
for (const auto &s : sec.sections) {
const auto t = find_topic(s, id);
if (t != nullptr) {
return t;
}
Expand All @@ -1142,13 +1109,13 @@ const topic *find_topic(const section &sec, const std::string &id)

const section *find_section(const section &sec, const std::string &id)
{
section_list::const_iterator sit =
const auto &sit =
std::find_if(sec.sections.begin(), sec.sections.end(), has_id(id));
if (sit != sec.sections.end()) {
return *sit;
return &*sit;
}
for (sit = sec.sections.begin(); sit != sec.sections.end(); ++sit) {
const section *s = find_section(*(*sit), id);
for (const auto &subsection : sec.sections) {
const auto s = find_section(subsection, id);
if (s != nullptr) {
return s;
}
Expand Down
23 changes: 4 additions & 19 deletions src/help/help_impl.hpp
View file
Expand Up @@ -49,15 +49,12 @@ class config;
class unit_type;
class terrain_type_data;
typedef std::shared_ptr<terrain_type_data> ter_data_cache;
namespace help { struct section; } // lines 51-51

namespace help {

/// Generate the help contents from the configurations given to the
/// manager.
void generate_contents();

typedef std::vector<section *> section_list;

/// Generate a topic text on the fly.
class topic_generator
Expand Down Expand Up @@ -136,6 +133,8 @@ struct topic
mutable topic_text text;
};

struct section;
typedef std::list<section> section_list;
typedef std::list<topic> topic_list;

/// A section contains topics and sections along with title and ID.
Expand All @@ -149,9 +148,6 @@ struct section {
{
}

section(const section&);
section& operator=(const section&);
~section();
/// Two sections are equal if their IDs are equal.
bool operator==(const section &) const;
/// Comparison on the ID.
Expand Down Expand Up @@ -193,8 +189,8 @@ class title_less
class section_less
{
public:
bool operator()(const section* s1, const section* s2) {
return translation::compare(s1->title, s2->title) < 0; }
bool operator()(const section& s1, const section& s2) {
return translation::compare(s1.title, s2.title) < 0; }
};

class string_less
Expand All @@ -205,17 +201,6 @@ class string_less
}
};

struct delete_section
{
void operator()(section *s) { delete s; }
};

struct create_section
{
section *operator()(const section *s) { return new section(*s); }
section *operator()(const section &s) { return new section(s); }
};

/// Thrown when the help system fails to parse something.
struct parse_error : public game::error
{
Expand Down
18 changes: 8 additions & 10 deletions src/help/help_menu.cpp
View file
Expand Up @@ -70,13 +70,12 @@ void help_menu::update_visible_items(const section &sec, unsigned level)
// Clear if this is the top level, otherwise append items.
visible_items_.clear();
}
section_list::const_iterator sec_it;
for (sec_it = sec.sections.begin(); sec_it != sec.sections.end(); ++sec_it) {
if (is_visible_id((*sec_it)->id)) {
const std::string vis_string = get_string_to_show(*(*sec_it), level + 1);
visible_items_.emplace_back(*sec_it, vis_string);
if (expanded(*(*sec_it))) {
update_visible_items(*(*sec_it), level + 1);
for (const auto &s : sec.sections) {
if (is_visible_id(s.id)) {
const std::string vis_string = get_string_to_show(s, level + 1);
visible_items_.emplace_back(&s, vis_string);
if (expanded(s)) {
update_visible_items(s, level + 1);
}
}
}
Expand Down Expand Up @@ -126,9 +125,8 @@ bool help_menu::select_topic_internal(const topic &t, const section &sec)
expand(sec);
return true;
}
section_list::const_iterator sit;
for (sit = sec.sections.begin(); sit != sec.sections.end(); ++sit) {
if (select_topic_internal(t, *(*sit))) {
for (const auto &s : sec.sections) {
if (select_topic_internal(t, s)) {
expand(sec);
return true;
}
Expand Down

0 comments on commit 06f0776

Please sign in to comment.