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

Reading a file and storing the contents in an array

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

Problem

This is my code for reading a file with a delimiter. Any suggestions to help improve my code efficiency?

I am not satisfied with using array data structure. Can any other data structure be used instead of an array?

bool read_file(char *p_file_name,char *file_content[FILE_NO_OF_ROWS][FILE_NO_OF_COL],const char *delimiter, int& token_count )
{

   using namespace std;
   ifstream file_is_obj;
   file_is_obj.open(p_file_name,ios::in);

   string buffer,token;
   int token_pos;
   token_count=0;

   if( file_is_obj.is_open())
   {
      while( !file_is_obj.eof() )
      {
         if(token_count >= FILE_NO_OF_ROWS)
         {
            break;
         }

         buffer.clear();

         token_pos=0;

         std::getline(file_is_obj,buffer);
         for( int iter=0;  token_pos  != -1 || iter >= FILE_NO_OF_COL ; iter++)
         {
            token.clear();

            token_pos=buffer.find(delimiter,0);
            token=buffer.substr(0,token_pos);
            if( !token.empty() )
            {
               buffer.erase(0,token_pos+strlen(delimiter));
               file_content[token_count][iter] = new char[token.length()+1] ;

               memset(file_content[token_count][iter],'\0',(token.length()+1)*sizeof(char));
               token.copy( file_content[token_count][iter], token.length(), 0);
            }
         }
         token_count++;
      }
      token_count-=1; // Token incremented for reading end of file
   }
   else
   {
      printf("\nFile not found (or) Error in opening the file");
      return false;
   }

   return true;
}

Solution

Some comments:

  • Use std::vector instead of an inflexible array. You're writing in C++, so you really ought to use the power of that language to write better code.



  • Don't use memset. It's generally wise to avoid mixing old-style C library functions such as memset, malloc and so forth, with C++. Strange things can happen and there's usually a better C++ way to do things without those.



  • Instead of using string.find, consider using other techniques such as the ones described here to split a line into its constituent pieces.



  • Rather than printing an error message, throw an exception. That way your code can be reused in situtations in which std::cout is not present or not visible.



  • Do NOT embed using namespace std; into code like that! It's extremely bad practice



EDIT:
Here's a quick alternative using a variation of the string split I pointed to in point #3.

#include 
#include 
#include 
#include 

template 
Container& split(Container& result,
  const typename Container::value_type& s,
  const typename Container::value_type& delimiters)
{
  result.clear();
  size_t current;
  size_t next = -1;
  do {
    current = next + 1;
    next = s.find_first_of( delimiters, current );
    result.push_back( s.substr( current, next - current ) );
  } while (next != Container::value_type::npos);
  return result;
}

int main(int argc, char *argv[])
{
    if (argc  > v;
    // fetch the file into v
    std::string line;
    std::vector fields;
    for( std::ifstream in(argv[1]); std::getline(in, line); ) {
        v.push_back(split(fields, line, delimiter));
    }
    // now print the results
    for (const auto &i : v) {
        for (const auto &j: i)
            std::cout << "[" << j << "]";
        std::cout << '\n';
    }
}


This uses C++11, but could easily be adapted for old compilers. Based on this input file:

alpha,frank,ruby,diamond
beta,joseph,emerald,circle
gamma,may,onyx,triangle


The code produces this output:

[alpha][frank][ruby][diamond]
[beta][joseph][emerald][circle]
[gamma][may][onyx][triangle]

Code Snippets

#include <iostream>
#include <fstream>
#include <string>
#include <vector>

template <typename Container>
Container& split(Container& result,
  const typename Container::value_type& s,
  const typename Container::value_type& delimiters)
{
  result.clear();
  size_t current;
  size_t next = -1;
  do {
    current = next + 1;
    next = s.find_first_of( delimiters, current );
    result.push_back( s.substr( current, next - current ) );
  } while (next != Container::value_type::npos);
  return result;
}

int main(int argc, char *argv[])
{
    if (argc < 2) {
        std::cout << "Usage readarray filename\n";
        return 0;
    }
    const std::string delimiter{","};
    std::vector<std::vector<std::string> > v;
    // fetch the file into v
    std::string line;
    std::vector<std::string> fields;
    for( std::ifstream in(argv[1]); std::getline(in, line); ) {
        v.push_back(split(fields, line, delimiter));
    }
    // now print the results
    for (const auto &i : v) {
        for (const auto &j: i)
            std::cout << "[" << j << "]";
        std::cout << '\n';
    }
}
alpha,frank,ruby,diamond
beta,joseph,emerald,circle
gamma,may,onyx,triangle
[alpha][frank][ruby][diamond]
[beta][joseph][emerald][circle]
[gamma][may][onyx][triangle]

Context

StackExchange Code Review Q#47784, answer score: 5

Revisions (0)

No revisions yet.