0

I'm learning C++, and have a question related to classes and templates. I know it's good practice to have "getters" for every variable in your class and a "setter" function. But with the code shown below, should you have a "getter" function for the array? In theory, the print function would serve the same purpose as the "getter" function would - print out everything in the array. If it is required, what would the correct code be to return that array? The array is an array of class objects.

My thinking and the code thus far:
queue.h

#pragma once


template<class T>
class Queue {
    public:
        Queue(int = 1);
        ~Queue();
        void setQueue(int);
        void enqueue(T);
        void dequeue();
        const T* getQueueArray() const;
        const int getArraySize() const;
        const int getArrayIndex() const;
        void printQueue();
    
    private:
        T* queueArray;
        int arraySize, arrayIndex;
};

queue.cpp

#include <iostream>
#include "queue.h"


template<class T>
Queue<T>::Queue(int arraySize) {
    this->setQueue(arraySize);
}


template<class T>
Queue<T>::~Queue() {
    delete [] this->queueArray;
}


template<class T>
void Queue<T>::setQueue(int arraySize) {
    this->arraySize = arraySize;
    delete [] this->queueArray;
    this->queueArray = new T[arraySize];
    this->arrayIndex = 0;
}


template<class T>
void Queue<T>::enqueue(T object) {
    if (this->arrayIndex == this->arraySize) {
        std::cout << "Rinda ir pilna, nevar pievienot elementu!\n";
    }
    
    else {
        this->queueArray[this->arrayIndex] = object;
        this->arrayIndex++;
    }
}


template<class T>
void Queue<T>::dequeue() {
    if (this->arrayIndex == 0) {
        std::cout << "Rinda ir tuksa!\n";
    }

    else {
        for (int i = 0; i < this->arraySize - 1; i++) {
            this->queueArray[i] = this->queueArray[i + 1];
        }

        this->arrayIndex--;
    }
}


template<class T>
const T* Queue<T>::getQueueArray() const {
    return this->queueArray;
}


template<class T>
const int Queue<T>::getArraySize() const {
    return this->arraySize;
}


template<class T>
const int Queue<T>::getArrayIndex() const {
    return this->arrayIndex;
}


template<class T>
void Queue<T>::printQueue() {
    for (int i = 0; i < this->arrayIndex; i++) {
        std::cout << i + 1 << ". ";
        this->queueArray[i].printHuman();
    }
}

The array getter function works and returns a memory address. Is that behavior correct?

And I'd like to ask another question, for class print functions, which would be the better of the 2:
std::cout << "something something" << classVariable; or std::cout << "something something" << getClassVariable(); One way is accessing the variables directly and the other is using the "getter" functions. Does it matter and does using functions like that impact performance in a noticable way?

Ervīns
  • 13
  • 3
  • 2
    Worth to read, because your code won't compile like that: [Why can templates only be implemented in the header file?](https://stackoverflow.com/questions/495021/why-can-templates-only-be-implemented-in-the-header-file) – Yksisarvinen Jan 18 '21 at 18:21
  • 1
    "I know it's good practice" should be "I know it's bad practice". Getters and / or setters only when they are essential. – QuentinUK Jan 18 '21 at 18:21
  • *I know it's good practice to have "getters" for every variable in your class* That's a misconception. Getters and setters are often signs of poor design. – n. m. could be an AI Jan 18 '21 at 18:23
  • I'll join the bandwagon. Re-evaluate this "good practice" http://www.idinews.com/quasiClass.pdf – StoryTeller - Unslander Monica Jan 18 '21 at 18:24
  • @Yksisarvinen #include "queue.h" and #include "queue.cpp" in main, compiles. – Ervīns Jan 18 '21 at 18:24
  • 2
    `#include "queue.cpp"` Don't do that. – n. m. could be an AI Jan 18 '21 at 18:25
  • Yeah, probably shouldn't have phrased it like that, it's just that the teacher at university taught it like that and generally when searching for code examples on the subject, they have the getters and setters. Naturally I assumed it was the industry practice, my bad. – Ervīns Jan 18 '21 at 18:26
  • @n. 'pronouns' m Is it really that bad? I had the problem of it not compiling and searched for the answer. That was one of the solutions proposed. I like to keep the implementation in a separate file, that's why I initially chose it. – Ervīns Jan 18 '21 at 18:30
  • 1
    The getter and setter methods allow accessing via interface. They hide the implementation. For example, instead of returning a hard-coded value, they could be changed to calculate the value and not break the interface. Direct access makes tighter coupling and increases dependencies between modules. – Thomas Matthews Jan 18 '21 at 18:31
  • 1
    You want to have getters for things that are actually part of the public usable API of the class and not for things that aren't. You're writing a queue, not an array. That means you access through something like a `top` or `front` method, not by handing people raw pointers to internal storage representations. – Nathan Pierson Jan 18 '21 at 18:32
  • 1
    Since a queue only allows you to add to one end and remove from the other, a getter providing access to all of the queue's data violates the definition of a queue. Not a good idea. – user4581301 Jan 18 '21 at 18:33
  • 1
    Templates are meant to go in headers. Headers are meant to be included in source files. Source files are meant to be compiled, not included in other source files. Do one thing wrong, and it forces you to do other things wrong too. Don't do it wrong in the first place. Why is it wrong if it works? Because you want to stick to something that works *always*, as opposed to this one time. Don't trust everything they tell you on the internet. – n. m. could be an AI Jan 18 '21 at 19:49

2 Answers2

0

First of all for templated classes, having the declaration in a header file and having the definitions on a source file wont work. So try putting them in the same header file.

I know it's good practice to have "getters" for every variable in your class and a "setter" function

Yes that is true but not always. Getting the actual data pointer will help for example when copying data to a buffer. Setting is usually done by assignment operators,

Value& operator=(const Object& other) { ... } // Copy assign operator.
Value& operator=(Object&& other) { ... } // Move assign operator.

So I would advice not to have a setter in these type of objects.

And I'd like to ask another question, for class print functions, which would be the better of the 2: std::cout << "something something" << classVariable; or std::cout << "something something" << getClassVariable();

This is opinion based and usually you print the content to the console by overloading the << operator,

template<class T>
ostream& operator<<(ostream& os, const Queue<T>& dt)
{
    os << /* print content */;
    return os;
}

int main()
{
    Queue<int> queue;
    std::cout << "Printing queue! " << queue;
}

One way is accessing the variables directly and the other is using the "getter" functions. Does it matter and does using functions like that impact performance in a noticable way

Nope they wont. If you inline the functions, there wont be no performance differences. And make sure you enable optimizations or else additional debug information would slow it down. I would argue that using a getter function would be safer.

D-RAJ
  • 3,263
  • 2
  • 6
  • 24
  • 1
    *Yes that is true but not always.* Misleading. The default should be no accessor. Accessors weaken encapsulation, just not as much as direct `public` access. Prefer functions that do something to the object rather than directly setting members or getting members and operating on their values outside of the object. – user4581301 Jan 18 '21 at 18:46
0

This post is largely opinion-based, but I'm going to offer my perspective. You've received comments telling you a few things.

First, I'd encourage you to do things "the C++ way". Templates are done in the header, and it's really gross to "#include "queue.cpp". But I understand at least for an example that's maybe okay, as it lets you focus on other things than all the code to implement the entire template. Just don't do it in practice.

As for getters and setters -- I disagree with some of the comments you're getting. I absolutely would NOT put a setter on the internal structure, but if this is a serious class, I'd absolutely implement some way to inspect the full contents.

Imagine, after all, that someone wants to be able to count how many elements in the queue have fulfill a particular constraint. You don't want to try to anticipate evreything someone might want to do. But you can certainly put some means of inspecting the contents, either by being able to get the current queue (const) or via iterators. Iterators are harder to implement, but they fit a lot of other algorithms. See the entire contents of #include .

So:

  1. Yes on some sort of access system (a getter / iterator)
  2. No on a setter

In addition to this, I'm not sure why your sample code isn't willing to grow the list if necessary, but if it's not going to, it should somehow indicate to the caller that it failed to enqueue properly.

Joseph Larson
  • 8,530
  • 1
  • 19
  • 36