Cursor module for a text-editor

A few weeks ago, I had started to write a text-editor in C++ using SFML library. The work is now paused pretty much because I am a lazy freak. So, I thought I might use this passive time to get reviews on what I have done so far.

As of now, I want to get reviews on the cursor module (for displaying a blinking cursor), I wrote for the text-editor. I am posting the code as it was written and it was written without many comments. (I am not sure if I am supposed to add comments before putting it up for review on the site.)

Cursor.h:

#ifndef CURSOR_H_INCLUDED
#define CURSOR_H_INCLUDED

#include
#include

class Cursor{
    private:
    sf::Color color;
    sf::RectangleShape cursorLine;
    sf::Clock clock;
    int col, line;
    int txtX, txtY;
    int maxCol;
    float thickness, length;
    int blinkPeriod, lastTime;
    bool visible;

    public:
    float getCol();
    float getLine();
    float getLength();
    float getThickness();
    void move(int, int);
    void setPosition(int,int);
    void setDefaultValues();
    Cursor();
    Cursor(float, float, float, float, int, int, int);
    void draw(sf::RenderWindow *);
    void update();
};

#endif // CURSOR_H_INCLUDED

Cursor.cpp:

```
#include "Cursor.h"

void Cursor::setDefaultValues(){
col = 1;
line = 1;
lastTime = 0;
thickness = 2;
length = 20;
blinkPeriod = 400;
visible = true;
color = sf::Color(0,255,0);
}

Cursor::Cursor(){
setDefaultValues();
}

Cursor::Cursor(float x, float y, float len, float thick, int txX, int txY, int maxCol){
setDefaultValues();
this->maxCol = maxCol;
txtX = txX;
txtY = txY;
col = x;
line = y;
length = len;
thickness = thick;
}

float Cursor::getCol(){
return col;
}

float Cursor::getLine(){
return line;
}

float Cursor::getLength(){
return length;
}

float Cursor::getThickness(){
return thickness;
}

void Cursor::move(int dx, int dy){
if(dx>0){
if(col+dx0) col +=dx;
else{

The names 'txtX' and 'txtY' don't say anything about the domain as 'line', or 'length' do. Someone would need to see how are you using that variable to figure out what it is.

If you want to make the code more readable, make explanatory variables and helper descriptive methods for example

bool fitsInCurrentLine = col+dx<=maxCol;
if( fitsInCurrentLine ) col += dx;
else moveToNextLine();

instead of

if(col+dx<=maxCol)col += dx;
else{
    col = 2;
    line += 1;
}

When I read the last one, I had to figure out "how" you were doing the thing to know "what" you were doing.

Note that I did not write the moveToNextLine method but you get the Idea of what is happening. Considering col = 2; line += 1;. Is it necessary to know that the initial col is 2? or Should I only know that you are moving to the next line if the line wraps?

It reads more like "well written prose" (as Uncle Bob in his book 'CleanCode' says)

You can do similar with the else block.

In the method setPosition, I think the correct name for the parameters is x and y instead of dx or dy given that they are not changes but just absolute values.

This if statement in the update method could be its own method

void Cursor::blink(){
    bool isTimeToChangeState = (clock.getElapsedTime().asMilliseconds()-lastTime)>blinkPeriod;
    if( isTimeToChangeState ){     
        visible = !visible;
        lastTime = clock.getElapsedTime().asMilliseconds();
    }
}

void Cursor::update(){
    cursorLine.setFillColor(color);
    cursorLine.setSize(sf::Vector2f(thickness, length));
    blink();
    cursorLine.setPosition((col-1)*(length*0.6)+txtX, (line-1)*(length*1.2)+txtY);
}

Finally, by reading

cursorLine.setPosition((col-1)*(length*0.6)+txtX, (line-1)*(length*1.2)+txtY);

you can sort of guess what is txtX. Is it the position of the text area? why just don't just put textAreaX and textAreaY, not perfect (I mean, for sure there should be a better name) but at least it improves a little the readability. In the book "clean code" you can find "A name that makes you go into the code to see that it tries to identify, is a bad name"

That's all I would recommend based on the book and experiences I've got as a developer. Happy coding!

StackExchange Code Review Q#160441, answer score: 4