Merging similar functions

Started by
3 comments, last by Zahlman 14 years, 10 months ago
Hi everyone, I've just wrote these two functions but they look terrible, it seems obvious to me that C++ must provide some way of having only one function with the same functionality. Any ideas on how I could transform them into a single function? Thanks in advance, Victor Freire

1. 
2.// How to merge these functions?
3. 
4. 
5.bool TextManagerClass::DrawText(const std::string &text, const int size, const int X, const int Y,
6.                              const int fR, const int fG, const int fB,
7.                             const int bR, const int bG, const int bB, const int alpha, 
8.                             const bool fit_on_screen, const bool line_break, const string &font_name,
9.                             SDL_Surface *screen)
10.{
11.        std::vector<std::string> newvec;
12.        newvec.push_back(text);
13.        return DrawLines(newvec, size, X, Y, fR, fG, fB, bR, bG, bB, alpha, fit_on_screen, line_break, font_name, screen);
14. 
15.}
16. 
17. 
18.bool TextManagerClass::DrawLines(const std::vector<std::string> &text, const int size, const int X, const int Y,
19.                              const int fR, const int fG, const int fB,
20.                              const int bR, const int bG, const int bB, const int alpha, 
21.                              const bool fit_on_screen, const bool line_break, const string &font_name,
22.                              SDL_Surface *screen)
23.{
24. 
25.( . . . )

Advertisement
imo they dont look that pleasant because of the number of params.
e.g. passing the rgb's as 2 vectors would make a much nicer header.
something like
bool drawText (string txt, int size, POINT p, Vector3 rgb, Vector4 rgba ...

calling a function superseeded by the other is fine practice.

you could wrap the both in a single DrawX function call with an extra argument 'what' or something..

draw (LINES, .... )
draw (TEXT, .... )

just switch on the value of the 'what' argument.

and also, some default parameter values may result in many shorter/nices calls...
Quote:Original post by Kincaid
imo they dont look that pleasant because of the number of params.
e.g. passing the rgb's as 2 vectors would make a much nicer header.
something like
bool drawText (string txt, int size, POINT p, Vector3 rgb, Vector4 rgba ...

calling a function superseeded by the other is fine practice.

you could wrap the both in a single DrawX function call with an extra argument 'what' or something..

draw (LINES, .... )
draw (TEXT, .... )

just switch on the value of the 'what' argument.

and also, some default parameter values may result in many shorter/nices calls...


There are a lot of parameters indeed. Hmm, I wanted to call the function in just one line but I wouldn't be able to do so using SDL_Color since it's a C struct with no constructors, had it been a class with an appropriate constructor the function call would be excellent.

DrawText("some text", 12, Point(44,22), SDL_Color(255, 255, 255), SDL_Color(255, 255, 255), ... );

Really? I thought doing that was outright inelegant.

Regarding the default parameters, there are some but they are not above because they are in the header file.

Thanks for the quick reply,
Victor Freire

Overloaded functions is what you're looking for I think. As long as your function parameters differ.

This would allow you to do something like this:

Draw("text", RGB(255,0,255), true, false);Draw(Vector2D(0,0), Vector2D(100,200), RGB(0,255,255));


Looks very elegant compared to the rest :D
0) What on earth is a TextManagerClass? First off, is it different from a TextManager? Adding "Class" to the end of a class name never really tells you anything. But then, "TextManager" is still quite vague. What does it mean to "manage" text?

Does the implementation actually use any member variables of the class? If not, it can be static. And if, as I suspect, it is static, and so is all the rest of the functionality, then what you really want is a namespace.

But don't just go that far. The OO thing to do is let some of the parameters for this monstrous thing become part of an object - let's say a "Font" - and have *it* do the drawing.

bool Font::Draw(const std::string& text, const Point& position,                bool fit_on_screen, bool line_break,                SDL_Surface* screen)// The size, name and colour information would have been previously passed to// a Font constructor, which would use the size and name to create and store// an SDL widget representing the font, and store the colours.// The Font does *not* contain the position information, render target, line-// wrapping options or the message itself, because those are not logically// part of the font abstraction.// Also, note that specifying 'const' on a parameter that is passed by value// is pretty much useless.


You don't have to stop there, either; you could have a Viewport abstraction that represents a region on an SDL_Surface into which text should be fitted, and then you can pass it a string and a Font, and it will make the appropriate Draw call on the passed-in Font.

1) You don't have to have different names for the two functions in C++; you can 'overload' the function instead.

2) Calling one version of the function from the other is a normal and reasonable thing to do. Trying to make a single function body work with either of two unrelated types (a string or a vector of string) is not. Not in C++, anyway.

3)
Quote:but I wouldn't be able to do so using SDL_Color since it's a C struct with no constructors, had it been a class with an appropriate constructor the function call would be excellent.


So make your own C++ struct version to wrap it.

// Implicit conversion, at the expense of slow initialization (can't initialize// the members of a base class directly in an initialization list).namespace alphanox {  struct Color : SDL_Color {    Color(int r, int g, int b) {      this->r = r; this->g = g; this->b = b;    }  };}// or// Faster initialization, at the expense of having to make a manual copy// to convert.namespace alphanox {  struct Color {    int r, g, b;    Color(int r, int g, int b) : r(r), g(g), b(b) {}    operator SDL_Color() {      SDL_Color result;      result.r = r; result.g = g; result.b = b;      return result;    }  };}// But the compiler might do some nice optimizations for you either way. I'd// prefer the first approach; it's simpler - even if arguably a little uglier.


And you could also add e.g. a single-int constructor expecting an ARGB-formatted color (or better yet, factory functions like colorFromARGB, colorFromABGR etc. so that it's clear what the input format is :) )

This topic is closed to new replies.

Advertisement