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

Small command-line helper tool

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

Problem

I've coded a small command line helper tool for this library I'm working on. The library provides tools for the use of Virtual Texturing on iOS devices (mainly games).

This little command line helper is still pretty much a prototype that I coded as quick as I could, for testing a new file format. Some feedback would be appreciated before I expand it further:

```
// Virtual Texturing Library:
#include "vt_tool_image.hpp"
#include "vt_tool_pagefile_builder.hpp"
#include "vt_tool_platform_utils.hpp"

// Standard library:
#include
#include
#include
#include
#include

namespace {

// ======================================================
// Local data:
// ======================================================

vt::tool::PageFileBuilderOptions cmdLineOpts;
std::string inputFile, outputFile;

// ======================================================
// printHelpAndExit()
// ======================================================

void printHelpAndExit()
{
std::printf("\n"
"Usage:\n"
"$ vtmake [--flags=]\n"
"\n"
"Flags accepted:\n"
"--help : prints help text with list of commands.\n"
"--filter : (str) type of mipmapping filter: box, tri, quad, cubic, bspline, mitchell, lanczos, sinc, kaiser.\n"
"--page_size : (int) total page size in pixels, including border.\n"
"--content_size : (int) size in pixels of page content, not including border.\n"
"--border_size : (int) size in pixels of the page border.\n"
"--max_levels : (int) max mipmap levels to generate.\n"
"--flip_v_src : (bool) flip the source image vertically.\n"
"--flip_v_tiles : (bool) flip each individual tile/page vertically.\n"
"--stop_on_1_mip : (bool) stop subdividing when mip 0 is reached.\n"
"--add_debug_info : (bool) print debug text to each page.\n"
"--dump_images : (bool) dump each page as an image file (TGA format).\n"
"--verbose : (bool) print stuff to STDOUT while running.\n"

Solution

-
Do not hardcode vtmake in printHelpAndExit Print argv[0] instead.

-
Each parseWhatever function has its own copy of code finding the =. This must be factored out into a function; even better, use strchr.

I would also recommend to restructure code to let parseCmdLine find the =.

-
The loop in parseCmdLine contains too much code. I would rather have a map of options to parsing functions. Similarly, I'd have a map of filter names to
vt::tool::FilterType values.

-
The overall command line structure is quite unconventional. Typically positional parameters follow options. Also it is very useful to make filenames optional as well: such tool can be then be used in a pipeline.

-
I don't see the need for cmdLineOpts, inputFile, outputFile to exist in a namespace scope. I recommend to define them in main and pass them to interested parties as parameters.

-
Finally, why not just use getopt?

-
A side note:

vt::tool::PageFileBuilder pageFileBuilder(inputFile, outputFile, cmdLineOpts);
pageFileBuilder.generatePageFile();


doesn't seem right. The PageFileBuilder looks like an implementation detail of generatePageFile, and should not be exposed to client.

Code Snippets

vt::tool::PageFileBuilder pageFileBuilder(inputFile, outputFile, cmdLineOpts);
pageFileBuilder.generatePageFile();

Context

StackExchange Code Review Q#70620, answer score: 3

Revisions (0)

No revisions yet.