How can I avoid repeating code with inline functions?

Question

I have repeating code in my work, and I want to get rid of it, so I know that in C++ it is not good idea to use macro, but instead I must use inline function, is it good idea to use this function as inline:

list<Data>::iterator foo(int data){
if(dataExists(data)){
    list<Data>::iterator i;
    for(i = dataClass.begin(); i != dataClass.end(); ++i){
       if(i->getData() == data){
        break;
       }
return i;   //here I have one more problem, what can I return if data doesn't exist?
}

I think that this function is very unsafe. How can I improve my code?

What actually want macro type functionality in C# or code improvement! — thatsalok, Jun 18 '10 at 09:32

score 8 · Accepted Answer · answered Jun 18 '10 at 09:32

8

What you're doing here is already done by the std::find() function, so it would be better to use that (although it's certainly OK to try implementing these things yourself for the exercise).

std::find() also demonstrates a good way to indicate the "not found" condition -- if the item is not found, it returns the iterator one-past-the-end. That way, the caller can determine whether a matching item was found by comparing the iterator returned with Data.end().

answered Jun 18 '10 at 09:32

j_random_hacker

50,331
10
105
169

6

Nope, there is not `std::list::find()`. But there is `std::find()` that you use like this: `std::find(myList.begin(), myList.end(), dataToFind)`. See http://www.cppreference.com/wiki/stl/algorithm/find – Ferdinand Beyer Jun 18 '10 at 09:38
1

One more thing: You'll have to `#include ` to use `std::find()` and friends. – Ferdinand Beyer Jun 18 '10 at 09:40
2

+1, but in this particular case you will need to use `find_if` together with a predicate that tests the equality of the value returned by the member function and the value you are looking for... – David Rodríguez - dribeas Jun 18 '10 at 10:26

score 4 · Answer 2 · answered Jun 18 '10 at 09:40

The code you posted makes no sense. at one point you use Data as a type, and at another as an object. Assuming it is an object, the way to indicate that something is not found would be to return an iterator pointing to its end.

return Data.end();

but your code is too confused for this to work without major changes.

You are not going to learn C++ by posting questions on SO - it simply cannot be done. Instead, you need to read a good C++ text book - I recommend Accelerated C++, which covers things like iterators in detail. If you carry on askking questions like this, you are simply wasting your time and, more importantly, ours.

score 1 · Answer 3 · edited May 23 '17 at 12:11

I don't really know what you are asking here, but there are several misconceptions in your post.

First inline functions are a specific optimization you do not need to care about quite yet. Learn about normal functions first and understand them properly.

As another answer already said, std::find() does what you seem to want to do. It doesn't have to be a member of list to work, in fact modern C++ styleguides often prefer non member functions.

Now to your code. I am pretty sure that the code you posted is not working C++ code, which makes it really hard to understand what you are trying to do. The list type you are using is also not a std::list<> (The iterators work differently), and you use Data as both a variable (which is not defined in your code) and as a type.

As someone already suggested maybe you should start a little easier, or better yet, get a good book (I recommend "Accelerated C++" and "Programming P&P using C++") for C++ beginners.

How can I avoid repeating code with inline functions?

3 Answers3