BerndGabriel/HtmlViewer — Issue #16: Various suggestions
Author: tetardd
Created: 2012-01-15 10:02:07+00:00
State: open

Hi,

First, of all, a big thnk you for this component, as I was looking for an HTML parser and now I found one. I just thought you might be interested in knowing how developers will use it and receive some feedback/suggestions on it.

I'm just interested in a component that can take some HTML text and render it on a canvas. Therefore, your component is much more powerful than my needs and I'm in the process of removing all the unecessary code (e.g. I need to be able to draw text with formatting, have images embedded but I don't need sounds, scripts, form controls, events). While doing this, I got to learn about the structure of your objects and would like to suggest the following:

I understand that in version 12, you are planning some in depth internal changes so this might be what you are planning already. This is my humble suggestion:
You could have 4 main core objects (and my suggestion mainly focuses here on number 2 and 3):
1- The parser: the one that takes an HTML file and fragments it into HTML tokens.
2- The renderer on a canvas: I would see this one as being the object that only handles graphical tokens (e.g. text formatting, lines, images, etc.) and it is a descendant of TObject and simply drawns on a provided canvas within a provided Rect (e.g. Procedure DrawHTML(aCanvas : TCanvas; aRect : TRect)). This can thus be used in places where a software only needs powerful drawing of formatted texts but no need for all the overheads associated with the more powerful HTML features (my case to be honest).
3- The renderer of full HTML features on a control: This is a descendant of the canvas renderer in point 2. It also handles creation of all form controls, sounds, events, etc. hance needs to render on a control (i.e. is kind of THTMLViewer but with a large part of the processing done by point number 2 moved to that component)
4- The HTMLViewer control: a control that can be placed on a form to display an HTML text.

I think this architecture would allow you to see your components used in many more applications, you might for example be aware of TMS Software for example where they have many numerous HTML renderers (ridiculous number actually). Using their components + yours could add 1 MB of exe size for codes aiming at doing the same thing. If you implemented my suggestion and manage to impose yourself as the HTML renderer standard in Delphi (as you could easily, your component is so nice), your code could be used by them and others, helping maintenance and code size.

Various other suggestions:
- Have you considered cleaning the code so that it adheres to coding standards in terms of naming? You probably know, for example that a "T" is usually used in front of all type names. This is missing in numerous instances. It makes it very difficult for people reading you code to understand when it's not clear immediately what word corresponds to what (a type, a variable?).

- In the same direction, it is not recommended to have variables in public section of objects, better move them to the private section and use a property:

TMyClass = Class(TMyAncestor)
Public
XXX : TXXXType;
...
End

to be replaced by:
TMyClass = Class(TMyAncestor)
Private
FXXX : TXXXType;
Public
...
Property XXX : TMyXXXType Read FXXX Write FXXX;
End

Also, it is common practice to call all private variables with an "F" prefix.

- Please, comment your code, indicate what each method is meant for. So far, very little in that direction.

All of these contribute to users being able to concentration on code meaning rather than syntax. As you clearly are focused on open source for other to use, that would bring a big benefit to the community.