1

I have lots of code like this:

otherString1 = myString1.replace("a", "b").replace("c", "d").replace("e", "f");
otherString2 = myString2.replace("a", "b").replace("c", "d").replace("e", "f");
otherString3 = myString3.replace("a", "b").replace("c", "d").replace("e", "f");

I would like to not repeat those replace methods again and again. What is the right approach to re-factoring of such code? I'm new to C++...

I thought I could do:

#define REPLACE .replace("a", "b").replace("c", "d").replace("e", "f")
otherString1 = myString1#REPLACE;

but this does not work.

I obviously cannot monkey-patch the string class to add myReplace()...

What to do? And should I put the replacement code into header or the sourse file? What about those static, inline, const things? Should I create a whole helper class and a helper method or should I create just a function somewhere? What about something like:

[helper.hpp]
static inline const myReplace(const StringClass s);

[helper.cpp]
static inline const myReplace(const StringClass s) {
    return s.replace("a", "b").replace("c", "d").replace("e", "f");
}

[somefile.cpp]
include "helper.hpp"
otherString3 = myReplace(myString3);
Tuom L.
  • 182
  • 3
  • 12

2 Answers2

5

IMO, you are overthinking it. Just create a function that takes a string (by const reference) and returns the modified string. Declare it in a header and define in the corresponding .cpp file.

Job done.

[helper.hpp]
std::string myReplace(const std::string& s);

[helper.cpp]
std::string myReplace(const std::string& s) {
   ...
}

[somefile.cpp]
#include "helper.hpp"
otherString3 = myReplace(myString3);
NPE
  • 486,780
  • 108
  • 951
  • 1,012
  • It should be a normal free function, not a method. – Pubby Mar 23 '13 at 15:16
  • Method of what? HelperClass? `const` reference - you mean `const StringClass &s`? – Tuom L. Mar 23 '13 at 15:17
  • Method of nothing, a free function in other words. Any reason for using `StringClass` not `std::string`? – john Mar 23 '13 at 15:20
  • And would it be better: `inline const std::string myReplace(const std::string& s)`? – Tuom L. Mar 23 '13 at 15:20
  • My compiler inlines function you didn't ask it to inline and refuses to inline functions you did ask it to inline. This is common with modern C++ compilers. So it makes very little difference either way. Apart from the fact that an inline function goes in the header file and a non-inline function goes in the cpp file. That really is what inline means. – john Mar 23 '13 at 15:21
  • @TuomL.: Frankly, to me it just looks like you are throwing all those keywords (`static`, `inline`, `const`) in without really understanding what they mean. I don't think this is a good strategy for writing code (or indeed learning a language). – NPE Mar 23 '13 at 15:21
1

I just want to point out that your macro would have worked, you just used it incorrectly. However, this is not the right way to solve this problem, just wanted to point it out. Here's the correct usage:

#define REPLACE .replace("a", "b").replace("c", "d").replace("e", "f")
otherString1 = myString1 REPLACE;

Or maybe better (if using macros can ever be better):

#define REPLACE(str) str.replace("a", "b").replace("c", "d").replace("e", "f")
otherString1 = REPLACE(myString1);

Remember, don't do this, but this is how macros could be used.

Xymostech
  • 9,710
  • 3
  • 34
  • 44
  • Ok, thanks. But could you please explain why this is not a good practice? – Tuom L. Mar 23 '13 at 15:27
  • 1
    @TuomL Because when you use macros, it is hard to tell what is really going to happen. Since they are literally text expansions of what you wrote, very bad things can happen in general. For example, see [here](http://stackoverflow.com/q/4079443/57318) and [here](http://stackoverflow.com/q/480939/57318) for some good examples. – Xymostech Mar 23 '13 at 15:32