Re: code improvements for the viewer



Pavel Tsekov wrote:
Just out of curiosity - can you outline the most important changes that
you are going to commmit.

The main change is that, for example in the view_display_text() function, coordinate computation had been done relative to the top left corner of the WView widget. One consequence was that the view_display_text() and view_display_hex() function had to know whether the status line is displayed (it always is) and which kind of ruler is active (try Alt-R in mcview; it can be invisible, at the top or at the bottom).

These calculations have been factored out into the function view_compute_areas(). Now while calculating the coordinates for where to display characters, (0,0) always means the upper left corner of the current area (which is one of view->status_area, view->ruler_area and view->data_ares). Only when calling view_gotoyx(), the (top,left) coordinates of each area are considered.

An example of code that gets much simpler is this:

view_display_text():
-           col = ((col - left) / 8) * 8 + 8 + left;
+           col = col - col % 8 + 8;

The new code just advances the column to the next tab stop. The old code first had to subtract the left coordinate of the viewer and had to add it at the end. Even worse, some revisions before it had not been called ``left'', but ``frame_size''.

Another thing is that many coordinate calculations were done incorrectly, but with valid results:

view_percent():
-    const int xpos = view->widget.cols - view->dpy_frame_size - 4;
+    const screen_dimen xpos = view->status_area.left
                             + view->status_area.width - 4;

The point is, the old code relies on properties of the complete widget (width and frame size), whereas the new code only needs data that has to do with the status area itself. That is, the code expresses what is really meant, and not something coincidentally equivalent.

Similarly, in view_status():
-    widget_move (view, view->dpy_frame_size,
                        view->dpy_frame_size + 24);
+    widget_move (view, top, left + 24);

In view_display_ruler(), there had been an off-by-one error which only occured in panel mode (that is: view->dpy_frame_size != 0):

-    for (c = left; c < right; c++) {
-       cl = c + view->dpy_text_column;
+    for (c = 0; c < width; c++) {
+       cl = view->dpy_text_column + c;

cl is the column in the current line (it is >= 0 if you scroll to the right). In the old code, it took into account whether the data was displayed with or without a frame. While separating the real calculation and the coordinate thing, I noticed that bug. It would have been hard to notice in the old code, as the calculation were more complex than necessary.

That's a small part of the ideas underlying my changes. If I had written everything, it would have been a book on programming style. :)

Roland



[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]