HiveBrain v1.2.0
Get Started
← Back to all entries
patterncMinor

ASCII art generator in C

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
artasciigenerator

Problem

I have written an ASCII art generator library. I was practicing data abstraction and code abstraction and I wanted to know if there is something that can be improved.

File tree:

| |--Makefile
|--fonts--|--StarStrips.c
| |--whimsy.c
|
| |--core.h
---|--include--|--StarStrips.h
| |--whimsy.h
|
|--Makefile


/fonts/Makefile

sharedlib: whimsy.o StarStrips.o
@echo Building the Shared Library; \
gcc -shared -fPIC -o ascii-arts.so whimsy.o StarStrips.o;
StarStrips.o : StarStrips.c
@echo building StarStrips; \
gcc -c -fPIC StarStrips.c -o StarStrips.o -std=c99 -I../include
whimsy.o : whimsy.c
@echo building whimsy; \
gcc -c -fPIC whimsy.c -o whimsy.o -std=c99 -I../include


/font/whimsy.c

#include
#include
#include
#include
#include
int whimsy_allowed(int c)
{
return islower(c) || c == ' ';
}

int whimsy_index (int c)
{
return islower(c)? c -'a'+1 :0;
}

int whimsy_init(void){
font_alloc(whimsy,10,27);
d_alloc(whimsy,space,' ',5,ARRAY({
" ",
" ",
" ",
" ",
" ",
" ",
" ",
" ",
" ",
" " }));
d_alloc(whimsy,a,'a',11,ARRAY({
" ",
" ",
" ",
" d888b8b ",
"d8P' ?88 ",
"88b ,88b ",
"
?88P'88b",
" ",
" ",
" " }));
d_alloc(whimsy,b,'b',11,ARRAY({
" d8b ",
" ?88 ",
" 88b ",
" 888888b ",
" 88P
?8b",
" d88, d88",
"d88'`?88P'",
" ",
" ",
" " }));
d_alloc(whimsy,c,'c',8,ARRAY({
" ",
" ",

Solution

-
fonts/Makefile

Each object depends only on the corresponding source. It means that header modifications wouldn't trigger recompilation. You may fix it by explicitly spelling out dependencies:

whimsy.o: whimsy.c whimsy.h core.h


In general it is a good habit to have dependencies auto generated: even in your not very complicated case it is easy to miss the core.h dependency. Take a look at -M family of gcc options.

-
include/whimsy.h

Do not define objects (like struct font whimsy) in the header file. You never know how many times the client would happen to #include "whimsy.h" in different places of their project. Better practice is to have the definition in the .c file, and declare it in the header as

extern struct font whimsy;


-
DRY?

Unfortunately, you didn't show your font file. Also, I'm 95% sure the init and print files are identical modulo font name. If I'm correct, you need to unify them, and have the unified version in core.c.

-
Allocation

Allocating glyphs dynamically and copying them from a static area looks like a waste for me (it would make sense should you read glyphs from the text file instead). I would have a

struct glyph {
    int width;
    char * appearance;
};


(strictly speaking, glyph.width is redundant: given a font height and appearance length you may calculate width at runtime); an array of glyphs as a part of struct font:

struct font {
    ....
    struct glyph typeface[128];
    ....
}


and a static initialization of each font like

static struct font whimsy = {
    ....
    .typeface = {
        ....
        ['a'] = (struct glyph) {
            .width = 10,
            .appearance =
                "          "
                "          "
                "          "
                " d888b8b  "
                "d8P' ?88  "
                "88b  ,88b "
                "`?88P'`88b"
                "          "
                "          "
                "          ";
        },
        ....    
    },
    ....
};


Beware that such partial array initialization is a gcc extension.

-
More abstraction

Now you may take advantage of appearance being default initialized to an NULL, and conclude that such glyph is not implemented. An allowed method becomes a trivial test, and is easily abstracted out of the font specifics.

Code Snippets

whimsy.o: whimsy.c whimsy.h core.h
extern struct font whimsy;
struct glyph {
    int width;
    char * appearance;
};
struct font {
    ....
    struct glyph typeface[128];
    ....
}
static struct font whimsy = {
    ....
    .typeface = {
        ....
        ['a'] = (struct glyph) {
            .width = 10,
            .appearance =
                "          "
                "          "
                "          "
                " d888b8b  "
                "d8P' ?88  "
                "88b  ,88b "
                "`?88P'`88b"
                "          "
                "          "
                "          ";
        },
        ....    
    },
    ....
};

Context

StackExchange Code Review Q#105611, answer score: 5

Revisions (0)

No revisions yet.