current community

your communities

more stack exchange communities

Stack Exchange
tour help
Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 1 down vote favorite

I want to create an adjacency list based on data I get from a text file. Could I make this simpler?

#include <iostream>
#include <fstream>
#include <sstream>

using namespace std;

struct Vertex;
struct Edges;

struct Edges
{
    // edges link the vertex
    Vertex * linkTo;
    // pointer to vertex node
    Edges * next;
};

struct Vertex
{
    int parent;
    int child1;
    int child2;
    int child3;
    int child4;
    int child5;
    int child6;
    int child7;
    int child8;
    Vertex * next;
    Edges * Links;
    // pointer to edges list
    void Add_data(Vertex * node);
};

class Graph
{
    public:
      Vertex vertices;
    // call the vertex struct to the Graph
    private:
      void BFS();
};

//-------------------------------- Function Implementation--------------------------

void Add_data(Vertex * node)
{
    ifstream file("Table.txt");
    string line;
    int parent;
    int child1;
    int child2;
    int child3;
    int child4;
    int child5;
    int child6;
    int child7;
    int child8;

    while(getline(file,line))
    {
        istringstream data(line);
        if(!(data>>parent>>child1>>child2>>child3>>child4>>child5>>child>>6>>child7>>child8))
        {
            cout << "Error " <<endl;
        }
        else
        {
            node->parent = parent;
            node->child1 = child1;
            node->child2 = child2;
            node->child3 = child3;
            node->child4 = child4;
            node->child5 = child5;
            node->child6 = child6;
            node->child7 = child7;
            node->child8 = child8;
            node->next = NULL;
        }
    }
}
share|improve this question
    
Please leave the code intact after receiving answers. –  Jamal♦ Apr 13 at 13:56
    
Why are you initializing so many variables? That could be streamlined immensely. Honestly, I would create an array to store the children and have them point to each other as necessary. It becomes very convoluted when you have massive if statements and variable assignments all over your code. –  Evan Bechtol Apr 13 at 14:00
1  
@mello This does not resemble an adjacency list in the form that I would consider correct. You may need to go back to the drawing board on this one! –  Evan Bechtol Apr 13 at 14:03
    
the reason i took down the post is that the Someone says it is not good... and @EvanBechtol i was trying to create adjacency list that read from file –  mello Apr 13 at 14:03
    
@mello Go to my profile and look at the "Torus Maze" question that I posted a few fays ago. It's in java, but I use an adjacency list in the code. It should give you an idea of how to better construct one. –  Evan Bechtol Apr 13 at 14:04

1 Answer 1

up vote 2 down vote
  1. You don't have enough comments, and the comments that you have don't explain anything. Comments are supposed to explain why the code is the way it is (in other words: what the goal is), not restate the obvious. Everyone who knows graph theory knows that edges link vertexes, and everyone who knows C knows that something typed as Vertex * is a pointer to a vertex. Those comments have no point and should be removed.
  2. Where's the implementation of Graph::BFS()?
  3. Where's the code that calls ::Add_data?
  4. What is ::Add_data even supposed to do? It appears to only support one-line files; was that your intention? Is it the same function as Vertex::Add_data?

  5. What's the point of the linked list, when it never gets initialized?

  6. Whenever you're manually repeating something, either by typing similar stuff over and over again, or by copy-pasting, you're doing something wrong. Computers are good at automating repetitive tasks. When you write code, you should think of writing loops. So code like 

    if(!(data>>parent>>child1>>child2>>child3>>child4>>child5>>child>>6>>child7>>child8))

    should never appear (besides the obvious syntax error of the spurious >> between child and 6).

share|improve this answer
    
so basically i am trying to push in the data from the text file to and adjacency list then do the BFS... –  mello Apr 12 at 23:11
    
But the code never builds the adjacency list, and I see neither the source code to BFS nor any calls to it, so I continue to be mystified. –  Snowbody Apr 13 at 1:24
    
so the way i have the adjacency list is not good right ? and can you should me an example if you can please –  mello Apr 13 at 13:43
    
I really don't see any adjacency list in your code at all. In order to have an adjacency list you'd need to store it as a std::list<>, a std::vector<>, or a std::array<>, or even as just an old style square-bracket array. Google for code if you want examples; we're not here to write code for you. –  Snowbody Apr 13 at 13:47
    
I have one more question ... is it the same way to push data from text file to an array when i am using STL list –  mello Apr 13 at 13:52

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.