Question

I have some technical 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'm beginner, and I think that this function is very unsafe, can somebody please give me advice, how can I improve my code, thanks in advance

P.S. what usually do You use to avoid repeating code?

I edited code

Was it helpful?

Solution

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().

OTHER TIPS

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.

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.

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top