accessors.

[Lynx876]

So, I have always used accessors in oop and never really given it any thought that they were bad, as I was taught to do it that way when I attended university.

Reading through a thread earlier, I came across a post by L B. So I checked on the internet to see the best way of keeping the encapsulation of a class.

Fair to say, I found more information on why they are "evil" and hardly anything "decent" on a good concept of creating a good class.

I came up with the following, hopefully I'm going in the right direction. I just want some information etc on how to do it better, maybe a link or two, of a good tutorial!

So, here's what I have:
main.cpp

#include "sampleClass.h"

#include <iostream>

int main()
{
	sampleClass myClass;

	myClass.setText();
	myClass.setInt();

	std::cout << "\nYour text is: ";
	myClass.displayText();

	std::cout << "\nYour integer is: ";     
	myClass.displayInt();

	return 0;
}

sampleClass.h

#pragma once

#include <iostream>
#include <string>

class sampleClass
{
	public:
		sampleClass( std::string t = "Uninitialized", int i = 0 );
		~sampleClass(void);

		// getters/setters
		/*void setText( const std::string &t )     { text = t; }
		std::string getText() const                { return text; }

		void setInt( const int &i )                { integer = i; }
		int getInt() const                         { return integer; }*/

		void setText()
		{
			std::cout << "Enter your text: ";
			std::getline( std::cin, text, '\n' );
		}

		void setInt()
		{
			std::cout << "Enter an integer: ";
			std::cin >> integer;

			clearBuffer();
		}

		void displayText()		{ std::cout << text; }
		void displayInt()		{ std::cout << integer; }
		
		void clearBuffer()
		{
			std::cin.clear();
			std::cin.ignore( std::numeric_limits< std::streamsize >::max(), '\n' );       
		}

	private:
		std::string text;
		int integer;
};

Thanks for any help/information. (:

[ne555]

Search for:
Tell, don't ask
SOLID
Design patterns (GoF)


It is not clear what your class is supposed to do.
it is too coupled to the UI
`clearBuffer()' shouldn't be a member function.
it is not const-correct

[Lowest0ne]

My opinion is that you want to separate the input method from the class, something more like:

cin >> text;
myClass.setText(text);

Which will allow you to set the text without caring about where it came from.

I do believe your example is a little too simple for your real question. Think about it in terms of what your class does. If you class is a Car, you wouldn't nessesarily want a setSpeed(), but an accelerate() and/or brakes(). These functions may have similar code, but you know what the class can do via the name.

class Car
{
  int speed;
  //...
};

class DrugDealer
{
  int speed;
  //...
}

int main(void)
{
  // Not very specific
  myCar.setSpeed(10);
  myDealer.setSpeed(10);

  // Spefic
  myCar.accelerate(10);
  myDealer.buyDrugs(10);
}

Just my opinion, might not be "so great".

I just think about c# which uses this format:

class Car
{
  int speed;

  int Speed
  {
    get { return speed;   }
    set { speed = value; }  // value is a c# keyword
  }
};

// which allows
car.Speed = 10;
std::cout << car.Speed << '\n';  // if you can use cout in c#

// This also allows you to bind this class to a System::Form
// Which will automatically call get/set when you have a text box or something.

// I think this would make a compile error, which is nice
car.Speed = "hello";

[Stewbond]

I wouldn't consider accessors to be evil at all (though I'm also not as good as L B at this stuff).

In this case, I don't see the point in using them.

class SomeClass
{
    int i;
public:
    int get() { return i; }
    void set(int in) { i = in; }
};

However in this case, I think they're perfect:

class SomeClass
{
    int i;
public:
    int get() { return i; }
    void set(int in) { i = (in > 5) 5 : in; }
}

In your case, you're using stdin and stdout in your class. In general that's a good thing to avoid. Let's say that you want to use your class in a Win32 program, or move it to a general purpose library. You'd have to strip out the console stuff. A class should have only one purpose, if it does two things, you should create another class.

[cire]

Stew, I think your second example shows the point in using them in the first. Without using them in the first, client code must change if you ever decide to go with the second whereas if they're used in the first, client code can remain the same for the second.

Accessors are not good or bad. They simply are. If they permeate your code, however, there is very likely a design issue that needs to be addressed. It is this "smell" about code with getters and setters that is off-putting to people. On the other hand they can be handy in simple situations like the one illustrated here by Stew, and they do provide encapsulation where direct access would be used instead.

[Lynx876]

First off, yes! Sorry, my example was very bad! lol. I had to go out and didn't have much time to write it out.

As for clearBuffer() not meant to be there, I just threw it in there instead of formatting another .cpp file. My bad!

Again, it was bad. I used the class methods to convey that the class should in fact take care of the data, rather than getters/setters.

From the little information I found on creating a 'good' class, I have this idea in my head that the class should be the primary source of control, over it's variables. i.e., as little interaction via getters/setters. Would I be correct in thinking this?

I made a tic-tac-toe game the other day, using classes. If anyone cares to take a look and give some insight as to whether I am using classes in a good manner, I'd appreciate it!
http://www.dropbox.com/sh/1rimb0pnxvu52r7/y5vtyDsCYd

@ne555.
Thanks. I shall look them up in a minute!.

@LowestOne.
I do tend to do what your example suggests, whereas I wouldn't need to set the speed etc. But then I find myself going "back to my roots" - so to speak. And then using settters for post creation.

@Stewbond.
I'm primarily in the console at the moment. Deciding whether to go down the road of Win32 or SDL. As mentioned above in my post, it was a bad example, and as being in the console, that's the first thing that popped in my head, to add in to the class' method, lol.

@cire.

cire wrote:

and they do provide encapsulation where direct access would be used instead.

For some reason, my tutor/lecturer at university always made us declare the variables as private and use setters/getters.

He was by no means a half decent teacher. But alas, I did learn a few things from him. albeit where to look on Google! aha!

And thanks a lot for the reply's!

[ne555]

//void GUI::initScreen()
   Timer pressToStart;

   pressToStart.setTime( GetTickCount() );

   pressToStart.setUpdate( 250 );
   pressToStart.setUpdate2( 650 );
   pressToStart.setToggle( true );

   while( ! _kbhit() )
   {
      pressToStart.setCurr( GetTickCount() - pressToStart.getTime() );

      if( ( pressToStart.getToggle() ) && ( pressToStart.getCurr() > pressToStart.getUpdate() ) )
      {
         pressToStart.setTime( GetTickCount() );

         con.write( pressKey, pressCenter, 21 );

         pressToStart.setToggle( false );
      }
      
      if( ( ! pressToStart.getToggle() ) && ( pressToStart.getCurr() > pressToStart.getUpdate2() ) )
      {
         pressToStart.setTime( GetTickCount() );

         con.write( blank, pressCenter, 21 );

         pressToStart.setToggle( true );         
      }

   }

   _getch();

//IIRC consider instead
   int t[] = {250, 650};
   int K=0;
   Alarm a(t[K]);

   while( ! _kbhit() )
      if(a.ring()){
         con.write( pressKey, pressCenter, 21 );
         K++; K%=2;
         a.program(t[K]);
      }

   _getch();

You only care about a `delayed activation'.
The alarm starts inactive, when `n' ticks pass, it activates and start ringing.
You can reprogram it (as such, you could use several deltas, you may want to encapsulate that behaviour in another class)

Note that `GetTickCount()' was moved to the timer implementation. So if you found that that function is no good, you can easily change it.