Good Programming practice ?

[kevinchkin]

Hi Guys,

Currently I am working on a project that involves extensive use of map, list, and vector.

Following some good programming practice, I have made sure that these variables are not global. Now, obviously I do want to access these containers in pretty much all of my functions. I have read somewhere, that we should always pass containers to other functions as references, because it causes lot of overhead to copy into local variables when passed as value. Is it so?

Also, in the scenario below:

class myClass { 
  private: 
     vector<string> myVec; 
  public: 
     vector<string> getVec()         ; 
     void           setVec(string s) ;
} ; 

Is it good idea to return private variable? I am not returning reference to them so I guess it should be all right or not ?

Last Question,

  map<string,myClass> myMap; 
  void accessVector(map<string,myClass>& myM); 

In the above code snippet, in the function accessVector. I want to access the vector returned from myClass. So in order to do that, should I pass in entire map to accessVector or should I just pass vector to it? Actually, to keep my main() clean, I am passing full map to the function. But I am not sure if that is a good practice.

Please guide me through.

Thanks

[Mythios]

I gather you never want to really copy a container. So wouldn't you be better of doing:

const std::vector<std::string>& getVec()
{
	return myVec;
}

Or something in the lines of that.

If your only going to need access to one of the vectors in the map your passing in. I personally think it'd be better off to just pass in a vector. As it would make more sense to what the function is doing.

[kevinchkin]

I gather you never want to really copy a container. So wouldn't you be better of doing:

const std::vector<std::string>& getVec()
{
	return myVec;
}

Thanks a lot for reply Mythios.
I thought that, we should never return reference to private members of class. Otherwise there is no point of them being private right.
That's why I am not returning reference to that member. However, I am not sure if what I think about not returning reference to private member is right or not. So, any advice would be very helpful.

Thanks

[Mythios]

I think you should always return a reference for a container.

For example :

#include <iostream>
#include <string>
#include <vector>

using namespace std;

class myClass
{
	private: 
		vector<string> myVec; 
	public: 
		myClass()
		{
			myVec.push_back("word1");
			myVec.push_back("word2");
			myVec.push_back("word3");
		}
		~myClass(){}
		const vector<string> getVec(); 
		void           setVec(string s);
};
const vector<string> myClass::getVec()
{
	return this->myVec;
}

int main()
{
	myClass m;

	for( unsigned int i = 0; i < m.getVec().size(); i++ )
		m.getVec()[i] = "FOO";
	return 0;
}

In main you would never actually be able to edit m.getVec()[i] = "FOO"; because it's returning it as const. This making it so you don't have to copy the container and you can't actually edit it.

There is nothing wrong with returning a private member. Just as long as you don't want it to be edited, but you have some other use for it. Etc wanting to copy it's elements to a newly created vetor.

[helios]

The point of making them private is to avoid accidentally corrupting the object's internal state by writing to something you shouldn't write to. Since the function's returning a reference to a const std::vector, there's no chance of that happening.

[Mythios]

That also won't compile as it's trying to edit a const return. If you remove the const you'll have full access to play with it. But yes in the example as wanting to copy the contents of the vector - I don't think there is to much harm in returning the private member.

There will be cases where you never will want to return something thats private. But depends if you do or don't need access to it outside the class.

[kevinchkin]

const reference is perfect. Thanks for the reply Mythios and Helios.

[Mythios]

No problem ;)

[jsmith]

The drawback to returning a reference to a private member is that it very slightly weakens encapsulation in that it would now become difficult to change the internal container without modifying the API also.

getVec() should be a const member function.

[jRaskell]

I'd take a step back and re-evaluate why your class needs to provide access to the entire private container. At most your class should implement an iterator that walks through the container, but by giving access to the entire container, you're tying interface and implementation together, and if you decide to switch to a different container type, your interface will have to change with the implementation.

[kevinchkin]

@jsmith:
Do you mean that getVec should be like this?
const vector<string>& getVec() const;

@jRaskell:
Actually class is user information. And it contains member const vector<string>& getFriends() const; which returns user's friends. User friends in vector<string> myFriends;
That's why I have to return private container from the function.

If what I am doing is incorrect then could you guys please give an example of better design for this?

Thanks

[jsmith]

Yes.

What jRaskell said was basically what I said.

It sounds like your class is a value class. [A value class is simply a container of data fields, much like a C
struct, without really much logic.] If that's the case, then it is OK to do what you are doing.

However, the very nature of a value class is that the class itself doesn't really manage the data; rather,
it simply contains it in the C++ scope sense, and other code is responsible for maintaining it. In these
cases, it is just as good to leave the data members public and eschew the setters and getters; they are
simply unnecessary syntactic baggage at that point, and do not add to data hiding/encapsulation.

[kevinchkin]

Thanks so much for clarifying jsmith.

I have one more question thou:
is it mandatory to initialize const_iterator ?

[jsmith]

I don't understand the question.

const_iterators have a default constructor that leaves the iterator in a useless state, so from a compile
standpoint, no, but from a usability standpoint you must explicitly assign a value to them before they
can be used.

[kevinchkin]

Sorry for not being clear. I was asking exactly what you explained. Thanks a lot jsmith.