...so naturally I turn to GameDev. In my class Node, I have two variables that I decided were too similiar:LinkedList<SyntaxData> syntaxData; and String text;(in my defense, syntaxData was added sometime after text.) Now SyntaxData is currently just a 'node' type that has a Pen color; and a String text;. The point behind it is that whenever text was changed(via accessor), the current lexer class would be called to parse the line and change syntaxData, which is what the OnPaint method uses to determine how the line should be highlighted with regards to syntax. But it occured to me that since Node was to be used per line of text(and there are usually several lines of text in code source), it wouldn't be a bad idea to remove text and just have the accessor Text 'put together' the broken string of text from syntaxData. However, the default lexer I'd written prior to thinking of removing text, PlainText, resets the syntaxData of the Node representing the line in question, and then adds one SyntaxNode containing the default color and the entire text of the line. Now, while I can easily change this without problem with something like LinkedList<SyntaxNode> temp=new LinkedList<SyntaxNode>();, it still leaves the problem of other lexers written by other people. My dilemma, then, is whether I should just leave String text; and ignore the redundancy per line, or remove the String text; and put in some warning that overwriting the syntaxData for the line will also overwrite the text of the line and pray nobody does it. Or perhaps there is a third(or more) option that I'm not immediately seeing that can solve my problem. Thanks in advance for your advice. Edit and P.S., I believe I forgot to mention that whenever the set part of the accessor Text is called, that is when the current lexer is called to parse the line. Not sure if its relevant or not...

It's much better to have that redundancy in the code than it is to allow some other unwitting programmer to break the whole thing completely! It isn't a bad idea to have both either, since text stores the source line you're parsing (which seems like a good idea to keep around), while syntaxData contains the parsed highlighted text.

You should always write your class interfaces to make it easiest for the users of the class. You should assume they will not read any documentation. Redundancy in your class should generally be extracted and combined into functions, but that's a different implementation detail.


Consider renaming those parts of your class so they're not confusing.

"Node", for instance, is too general, it's only a good name if it is embedded in something. There are lots of things that could be considered nodes. Names like 'color' and 'text' can possibly make sense on their own, but might also make more sense if accompanied by a noun or verb that clarifies their use.

If your value is temporary and will be overwritten, the name of the variable could indicate this.

Quote:Original post by frob
"Node", for instance, is too general...

It's technically CodeLite.Node, so it isn't entirely too general.