patterncppMinor
LaTeX table generator - file in argument list
Viewed 0 times
fileargumentlatexgeneratorlisttable
Problem
My code generates a LaTeX table from a plain text file which uses whitespace as delimiters. It asks you the title of the columns one-by-one (mind the formatting!). Then it outputs a
How can I improve it?
Here's the code:
latex.txt file with a nicely formatted table inside.How can I improve it?
Here's the code:
#include
#include
#include
#include
int main(int argc, char* argv[])
{
if(argc!=2){
std::cerr > matrix;
while(!fin.eof()){
std::vector v;
std::string line;
getline(fin, line);
std::istringstream nums(line);
double x;
while(nums >> x){
v.push_back(x);
}
matrix.push_back(v);
row++;
}
// Basic.
row--;
col = matrix[0].size();
std::ofstream fout("latex.txt",std::ios::out);
fout << "\\begin{center}\n\\begin{tabular}{";
for(int i=0;i<col;i++){
fout << "|c";
}
fout << "|}\n";
fout << "\\hline\n";
// Ask for col titles:
for(int i=0;i<col;i++){
std::cout << "What is the title of the " << i+1 << ". column?(Please mind the formatting!)" << std::endl;
std::string title;
getline(std::cin,title);
fout << title;
if(i!=col-1){
fout << " & ";
}else{
fout << " \\\\\n\\hline\n";
}
}
for(int i=0;i<row;i++){
for(int j=0;j<col;j++){
fout << matrix[i][j];
if(j!=col-1){
fout << " & ";
}else{
fout << " \\\\\n\\hline\n";
}
}
}
fout << "\\end{tabular}\n";
fout << "\\end{center}";
return 0;
}Solution
Argument checking
You test that the user has specified exactly one input file, but if not, then print the misleading message
Error loading file.
You would be better with a usage message, such as
Well done for correctly using
File checking
Here, you use the supplied filename, but never check whether
Similarly for
The reading loop also doesn't check for errors, and it has the
At the very end of the program, check the fail bit of
Input values checking
If there were no values, you might want to fail before starting to write the output:
I'm not sure whether the input needs to be rectangular (i.e. all rows have the same number of columns). If so, test it:
That
Overall approach
You probably don't want to be typing column headings each time you update the data file, so you're most likely to redirect stdin from a file (and you may well want to automate with Make or similar). I'd suggest that you accept two filename arguments - column headings first, one per line; numeric data second. That way, you can use the number of lines of the first file as the expected column count when reading the data file (and you could error early when reading a non-conforming line).
Reading the column names first gives you another potential advantage: you could transform the input to output line by line, rather than having to store all lines to the end. This makes the program a much better pipeline player, and is a good practice when the inputs might be very large.
You might consider simply writing the output to standard out, rather than opening
My version
I've only compiled this, not run it. In particular, I may well have bad logic in reading the double values from the line string, as I don't normally read whole lines and then parse them. I've made a couple of other changes, such as reducing scope of temporary variables, closing
```
#include
#include
#include
#include
int main(int argc, char* argv[])
{
if (argc!=2) {
std::cerr > matrix;
// Read input values into matrix
{
std::string line;
while (getline(fin, line)) {
std::vector v;
std::istringstream nums(line);
double x;
while (nums >> x) {
v.push_back(x);
}
if (!nums) {
std::cerr << argv[1] << ": parse error" << std::endl;
}
matrix.push_back(v);
}
}
if (!fin) {
std::cerr << argv[1] << ": reading failed" << std::endl;
return 1;
}
fin.close();
// Test that input isn't empty
const auto rows = matrix.size();
if (rows == 0) {
std::cerr << argv[1] << ": empty" << std::endl;
return 1;
}
// Test that input is rectangular
const auto cols = matrix[0].size();
for (auto i = cols/cols; i < matrix.size(); ++i) {
if (matrix[i].size() != cols) {
std::cerr << argv[1] << ":" << (i+1)
<< ": line has " << matrix[i].size() << " columns; expected " << cols << std::endl;
return 1;
}
}
auto const& outfile = "latex.txt";
std::ofstream fout(outfile, std::ios::out);
if (!fin) {
std::cerr << outfile << ": not writeable" << std::endl;
return 1;
}
static
You test that the user has specified exactly one input file, but if not, then print the misleading message
Error loading file.
You would be better with a usage message, such as
std::cerr << "Usage: " << argv[0] << " filename" << std::endl;Well done for correctly using
cerr for such messages.File checking
Here, you use the supplied filename, but never check whether
fin was successfully opened. You probably want:std::ifstream fin(argv[1],std::ios::in);
if (!fin) {
std::cerr << argv[1] << ": not readable" << std::endl;
return 1;
}Similarly for
fout:std::ofstream fout("latex.txt", std::ios::out);
if (!fin) {
std::cerr << "latex.txt" << ": not writeable" << std::endl;
return 1;
}The reading loop also doesn't check for errors, and it has the
while (!eof()) antipattern. This should be something more like (untested):std::string line;
while (getline(fin, line)) {
std::vector v;
std::istringstream nums(line);
double x;
while (nums >> x) {
v.push_back(x);
}
if (!nums.eof()) {
std::cerr << argv[1] << ": parse error" << std::endl;
}
matrix.push_back(v);
row++;
}
if (!fin) {
std::cerr << argv[1] << ": reading failed" << std::endl;
return 1;
}At the very end of the program, check the fail bit of
fout:fout << "\\end{tabular}\n";
fout << "\\end{center}";
return !fout;Input values checking
If there were no values, you might want to fail before starting to write the output:
const auto rows = matrix.size();
if (rows == 0) {
std::cerr << argv[1] << ": empty" << std::endl;
return 1;
}I'm not sure whether the input needs to be rectangular (i.e. all rows have the same number of columns). If so, test it:
const auto cols = matrix[0].size();
for (auto i = cols/cols; i < matrix.size(); ++i) {
if (matrix[i].size() != cols) {
std::cerr << argv[1] << ":" << (i+1)
<< ": line has " << matrix[i].size() << " columns, expected " << cols << std::endl;
return 1;
}
}That
cols/cols is just a shorter way to write (std::vector::size_type)1.Overall approach
You probably don't want to be typing column headings each time you update the data file, so you're most likely to redirect stdin from a file (and you may well want to automate with Make or similar). I'd suggest that you accept two filename arguments - column headings first, one per line; numeric data second. That way, you can use the number of lines of the first file as the expected column count when reading the data file (and you could error early when reading a non-conforming line).
Reading the column names first gives you another potential advantage: you could transform the input to output line by line, rather than having to store all lines to the end. This makes the program a much better pipeline player, and is a good practice when the inputs might be very large.
You might consider simply writing the output to standard out, rather than opening
latex.txt; that gives you more flexibility in how you use it. You'd have to not prompt for the headings then, though!My version
I've only compiled this, not run it. In particular, I may well have bad logic in reading the double values from the line string, as I don't normally read whole lines and then parse them. I've made a couple of other changes, such as reducing scope of temporary variables, closing
fin as soon as we've finished reading, and sprinkling const fairy-dust where I can.```
#include
#include
#include
#include
int main(int argc, char* argv[])
{
if (argc!=2) {
std::cerr > matrix;
// Read input values into matrix
{
std::string line;
while (getline(fin, line)) {
std::vector v;
std::istringstream nums(line);
double x;
while (nums >> x) {
v.push_back(x);
}
if (!nums) {
std::cerr << argv[1] << ": parse error" << std::endl;
}
matrix.push_back(v);
}
}
if (!fin) {
std::cerr << argv[1] << ": reading failed" << std::endl;
return 1;
}
fin.close();
// Test that input isn't empty
const auto rows = matrix.size();
if (rows == 0) {
std::cerr << argv[1] << ": empty" << std::endl;
return 1;
}
// Test that input is rectangular
const auto cols = matrix[0].size();
for (auto i = cols/cols; i < matrix.size(); ++i) {
if (matrix[i].size() != cols) {
std::cerr << argv[1] << ":" << (i+1)
<< ": line has " << matrix[i].size() << " columns; expected " << cols << std::endl;
return 1;
}
}
auto const& outfile = "latex.txt";
std::ofstream fout(outfile, std::ios::out);
if (!fin) {
std::cerr << outfile << ": not writeable" << std::endl;
return 1;
}
static
Code Snippets
std::cerr << "Usage: " << argv[0] << " filename" << std::endl;std::ifstream fin(argv[1],std::ios::in);
if (!fin) {
std::cerr << argv[1] << ": not readable" << std::endl;
return 1;
}std::ofstream fout("latex.txt", std::ios::out);
if (!fin) {
std::cerr << "latex.txt" << ": not writeable" << std::endl;
return 1;
}std::string line;
while (getline(fin, line)) {
std::vector<double> v;
std::istringstream nums(line);
double x;
while (nums >> x) {
v.push_back(x);
}
if (!nums.eof()) {
std::cerr << argv[1] << ": parse error" << std::endl;
}
matrix.push_back(v);
row++;
}
if (!fin) {
std::cerr << argv[1] << ": reading failed" << std::endl;
return 1;
}fout << "\\end{tabular}\n";
fout << "\\end{center}";
return !fout;Context
StackExchange Code Review Q#157567, answer score: 4
Revisions (0)
No revisions yet.