This is the fourth part in a series of articles in which I’m ‘highlighting’ what I consider to be lousy code provided by IAR in their NXP LPC1764 development kit. This example is taken from the VirtualCom project but can be found in almost all of the projects.
This code excerpt is taken from a file ‘Terminal_18_24x12.c”.
/*************************************************************************
*
* Used with ICCARM and AARM.
*
* (c) Copyright IAR Systems 2006
*
* File name : Terminal_18_24x12.c
* Description : Font Terminal 18 (24x12)
*
* History :
* 1. Date : December 2, 2006
* Author : Stanimir Bonev
* Description : Create
*
* $Revision: 28532 $
**************************************************************************/
#include "drv_glcd.h"
static const unsigned char TextStream [] = {
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
... One thousand five hundred and thirty+ more lines of data without a comment
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
};
FontType_t Terminal_18_24_12 =
{
12,
24,
0,
255,
(pInt8U)TextStream,
"Terminal 18 (24x12)"
};
Other than the header shown above, there is not a single comment in a 1566 line file. To help complete the picture, here is the excerpt from the header file drv_glcd.h which defines the data type FontType_t.
typedef struct _FontType_t {
Int32U H_Size;
Int32U V_Size;
Int32U CharacterOffset;
Int32U CharactersNuber;
pInt8U pFontStream;
pInt8U pFontDesc;
} FontType_t, *pFontType_t;
I have not omitted the comments – there just aren’t any.
Now it’s reasonably clear that this file is used to define a font. To illustrate my complaint with this file, I will pose what I consider to be a quite reasonable situation.
The sales department comes to engineering and says “90% of our sales for this product are in Germany. We have received complaints from Germany that the digit seven is rendered incorrectly – can you please correct it?” (For those of you that don’t know, in Germany a ‘7’ is usually rendered with a horizontal bar through the body. I would demonstrate it – but I can’t within the limitations of my editing environment).
Now the question is, how big a job should this be – and how big a job will it actually be based upon how the font has been implemented? I will let you the reader decide the answer to the first question. To the second question however, I will suggest an answer of days. The reason is that before I can possibly make the change I have to get the following questions answered:
- How is TextStream[] indexed? It’s likely by ASCII number – but who knows for sure?
- Just how many bytes are there per character?
- How do the bytes map onto the character – by row or by column and do they start at the bottom, the top, the left or the right?
- What about italics and bold? Are they included in this font definition or not? If not, is there a separate file that defines them, or is it done algorithmically?
- Is there a German style ‘7’ already in the font map that I could just use?
All of these questions could have and should have been answered in the font file. If it was me, I would have done it via a combination of coding changes and comments. For example I might have done something like this:
static const unsigned char TextStream [N_TYPES][N_CHARS][FONT_SIZE_IN_BYTES] =
{
/* Start of standard fonts (c.f. bold and italic) */
{
/* Character 0. This is the nul character and is non-printing.
Appears so as to make indexing into TextStream[][] easy */
{
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
...
Would this have been a lot of work up-front? Absolutely. However compared to what the poor sod that has to maintain the font has to go through, it would have been time well spent. Unfortunately the industry seems to measure productivity based on how quickly code is written – and not on how easy it is to maintain. I for one would like to see it changed!