JavaFX custom control - editable label

I have a few custom components, some bigger ones and some smaller ones. I'd like to know if the structure is correct. As in: Is the code to do x in the right places. For starters, I'd just like to show a very small one, take your feedback and adjust my other components accordingly.

It can be found here.

Before you click on the GitHub repo and have a look at it:

It's just very small adjustments made to TextField to make it feel like an editable label. I know that there is a lot more missing to make it a true editable label, but for that I'd have to extend a base class closer to control itself. This question is purely regarding the custom component structure in general + this implementation of EditableLabel fits all that I need it to do, that's why I chose to not go deeper into the rabbit hole with that.

• Did I structure the methods and functionality into the correct classes (control, skin, behavior)? What could I have done better there?

• Should I do more comments? Could you give me an example (or more)?

• Are there any monstrously wrong implementations in there?

• Are there smaller mistakes in the implementations?

EditableLabel.java

```
package com.github.rjwestman.editableLabel;

import javafx.beans.property.*;
import javafx.scene.control.Skin;
import javafx.scene.control.TextField;

import java.net.URL;

/**
* A TextField, that implements some Label functionality
*
* It acts as a Label, by removing the TextField style and making it non-editable.
* It is also not focus traversable.
*
* When clicking on it, it will switch to editable mode
* Changing focus away from the EditableLabel or pressing ENTER will save the changes made and deactivate editable mode.
* When pressing ESC it will exit editable mode without saving the changes made.
*
* @sa EditableLabelSkin, EditableLabelBehavior
*/
public class EditableLabel extends TextField {

/**
*

Comments and Javadoc

I really like your Javadoc and this is something I rarely say/see. It's clear to the point and does not feel like it does not belong there. What I don't personally like is think like this :

/************************************************************************
 * @}                                                                   *
 *                                                                      *
 * \defgroup StylesheetRelated                                          *
 *                                                                      *
 * @{                                                                   *
 ***********************************************************************/

I guess this is to separate part of the code into logic unit that you can probably either expand/hide with a plugin. The problem I have is it take a lot of place and it "itch" my eyes. The real deal breaker of your comments habit is :

// editableState change to not editable
                editableState = false;

The comment is really just repeating what the code is doing. Your code is really good and readable, with clear variable name, I really don't need the comment to understand the piece of code. This is happening a couple of times in your code. While it's not critic, it's taking space and make your class bigger for no real benefit. If you have to use those kind of comments, try to explain why the code is written like that, not what the code is. Every comment should add value.

One line method

This is not something I like that you're doing. While it's perfectly accepted and will compile just fine, I always feel it sacrifice readability for space.

@Override
protected Skin createDefaultSkin() { return new EditableLabelSkin(this); }

I've read a good deal of code and normally one line code is a variable declaration, a method call, an if declaration or something like that. It's not a method declaration. What it's doing is I'm quickly scanning your classes and I think there is a tons of variables (which would be weird) when in fact it was a method declaration.

private StringProperty baseText;
public String getBaseText() { return baseText.get(); }
public StringProperty baseTextProperty() { return baseText; }
public void setBaseText(String baseText) { this.baseText.set(baseText); }

I had to look carefully to see that you "packaged" the setter and getter in the same area that you're variable declaration. I'm not used to mixing class variable with methods. Does it imper readability? I would say yes. Would everyone say the same probably not. What I'm used too is something like :

public class Example {
    //variables
    private Long id;
    private String name;

    //methods of the class (no getter/setter)
    @Override
    protected Skin createDefaultSkin() { 
        return new EditableLabelSkin(this); 
    }       

    //getter and setter at the end since normally there nothing special
    public Long getId() {
        return id;
    }

    public void setId(Long id) {
        this.id = id;
    }

    public String getName() {
        return name;
    }

    public void setName(String name) {
        this.name = name;
    }

}

This make it easy to see what my class use as internal data, dependencies on others services, etc. Second thing I will read is what I can do with the class. I can quickly see what is the internal function of the class. At the end of the class, I'll have all my setters/getters since most of the times there is nothing special about. I'll probably move up getters and setters with special comportment.

Conclusion

Your code is really of great quality. Everything I had to say was about presentation rather than the implementation. To be honest, I did not your component to see if everything was correct, but from the look of it it's fine. I think you did a great job, but someone with more experience with JavaFX could comment on the implementation. The only thing I would add is that you could change "editable" to a constant and other String like this ("editablelabel.css" and "editablelabel.css").

StackExchange Code Review Q#120748, answer score: 2