Creating a Linear Linked List with an External File

[chmstry]

Hi, I think my problem may be quite minor; however, I am going to provide a bit of background information.

I am trying to read/load data from an external text file. The file contains information on mobile apps. It contains the name of an app, a description of an app, and a number of keywords followed by a list of keywords that will be used to categorize an app.

Example (in the file):
Words for Friends:A crossword puzzle game:3:puzzle:multiplayer:crossword:

I've created a class called MobileApp that will be used to manage one instance of a MobileApp. Also, I've created a structure called key that will be used in conjunction with a head pointer to a list of keywords, which is part of the MobileApp class:

struct key {
    char * keyword;
    key * next;
};

class MobileApp {
    public:
        MobileApp();
        ~MobileApp();
        void createApp(char * appName, char * appDescription, int numKeywords,
                       char ** keywords);
        int copyApp(MobileApp & copy);
    private:
        char * appName;
        char * appDescription;
        int numKeywords;
        key * head;
};

An instance of a MobileApp contains the name of the app, its description, and a linear linked list of keywords (note: each instance has its own linear linked list). I want to read each instance from a text file and then create a linked list of those instances - a linked list of MobileApps.

I have a separate class that deals with reading in from the file and creating the linked list of MobileApps. So far, I have successfully read in each MobileApp instance. In my read function, after I load the data into temp variables, I call my createApp function to set the data members. It works fine. However, I cannot figure out how to correctly write my copyApp function (see below). I do not know what to do for the linear linked list of keywords.

void MobileApp::createApp(char * appName, char * appDescription, int         
                          numKeywords, char ** keywords)
{
    //deep copy character arrays
    //etc.

    //shallow copy
    this->numKeywords = numKeywords;
  
    //linear liked list of keywords
    for(int i = 0; i < this->numKeywords; ++i) {
        if(!head){
            head = new key;
            head->keyword = new char[strlen(keywords[i]) + 1];
            strcpy(head->keyword, keywords[i]);
            head->next = NULL;
        }
        else{
            key * current = head;
            head = new key;
            head->keyword = new char[strlen(keywords[i]) + 1];
            strcpy(head->keyword, keywords[i]);
            head->next = current;
        }
    }
  
}

int MobileApp::copyApp(MobileApp & copy)
{
    //deep copy character arrays
    //etc.

    //shallow copy
    numKeywords = copy.numKeywords;

    //linear linked list of keywords
    //WHAT DO I DO HERE?
    char ** keywords = num char*[numKeywords];
    for(int i = 0; i < numKeywords; ++i) {
        if(!head){
            head = new key;
            keywords[i] = copy.head->keyword;
            head->keyword = new char[strlen(keywords[i]) + 1];
            strcpy(head->keyword, keywords[i]);
            head->next = NULL;
        }
        else{
            key * current = head;
            head = new key;
            keyword[i] = copy.head->keyword;
            head->keyword = new char[strlen(keywords[i]) + 1];
            strcpy(head->keyword, keywords[i]);
            head->next = current;
        }
    }

    return 1;

}

Any suggestions?? I know this is long winded, but I really need help on what to do. I don't think this is a large problem but for some reason I just cannot get it. I am just copying the same keyword over, I am not copying all the keywords and I don't know what to do to remedy this problem.

[kbw]

It seems like you're making a rod for your own back. If you're writing in C, then so be it. But in C++, use std::string for strings unless you have specific reasons to use something else.

C++ also has standard containers, amongst which is a double linked list, std::list, and a single linked list, std::forward_list. C++ programmers no longer write linked lists, dictionaries and so on, they're standard components.

Also, use const where appropriate. It'll stop you hurting yourself.

Anyway, back to your code.

You should remember that classes in C++ are the mechanism for creating user defined types. These user defined types should work just like built in types. To make your user defined types work like built in types, there are some members that are needed and are generated if you don't define them. They are:
* default constructor
* copy constructor
* assignment operator
* destructor

C++11 adds:
* move constructor
* move operator

These make your code work as you'd expect:

{
    MobileApp app("Words for Friends", "A crossword puzzle game", {"puzzle", "multiplayer", "crossword"});
    MobileApp d1(app);
    MobileApp d2;
    MobileApp d3;
    d3 = d2 = d1;
    std::vector<MobileApp> v { app, d1, d2, d3 };
}

Line 2: make a MobileApp, app, from inputs.
Line 3: make a MobileApp, d1, from app.
Line 4: make an empty MobileApp, d2.
Line 5: make an empty MobileApp, d3.
Line 6: assign the content of d1 to d2 and d3.
Line 7: make a vector v that is initialised with: app, d1, d2, d3
Line 8: cleanup app, d1, d2, d3, v

To make that happen, MobileApp should look like:

class MobileApp
{
public:
    MobileApp();
    MobileApp(const std::string& name, const std::string& desc,
        std::initializer_list<string> l);
    MobileApp(const MobileApp& n);
    MobileApp& operator=(const MobileApp& n);
    ~MobileApp();
    //... 

The code is the same was what you have, but organised such that MobileApp is a user defined type and can be used interchangeably with built in types.

Now, your copy function.

struct key
{
    key(const char* s) : keyword(strdup(s)), next(nullptr) {}
    ~key() { free(keyword); }
    key* clone() const { return new key(keyword); }

    char* keyword;
    key* next;
};

void copylist(key&* head, const key* c)
{
    if (c)
    {
        copylist(head, c->next);
        head->next = c->clone();
    }
}

void clearlist(key&* head)
{
    if (head)
    {
        clearlist(head);
        delete head->next;
    }
}

int MobileApp::copyApp(MobileApp & c)
{
    free(appName);
    free(appDescription);
    clearlist(head);

    appName = strdup(c.appName);
    appDescription = strdup(c.appDescription);
    numKeywords = c.numKeywords;
    copylist(head, c.head);
}

To rewrite your class in a standard way:

class MobileApp
{
public:
    MobileApp(
        const std::string& appName,
        const std::string& appDescription,
        std::initializer_list<std::string> keywords);
    MobileApp() = default;
    ~MobileApp() = default;
    MobileApp(const MobileApp& n) = default;
    MobileApp& operator=(const MobileApp& n) = default;

private:
    std::string appName;
    std::string appDescription;
    std::forward_list<std::string> keywords;
};

MobileApp::MobileApp(
        const std::string& appName,
        const std::string& appDescription,
        std::initializer_list<std::string> keywords) :
    appName(appName),
    appDescription(appDescription),
    keywords(keywords)
{
}

That's much simpler than messing around with pointers, right?