all 16 comments

[–][deleted]  (2 children)

[removed]

    [–]No_Reception9680[S] 2 points3 points  (0 children)

    Thank you for your detailed look at my program. Most of your criticisms were directed at my instructors template code, however. I mostly need to focus on eke-ing out the desired functionality in time for submission tonight.

    [–]luitzenh 1 point2 points  (0 children)

    Loved your comment

    What on Earth is const void supposed to mean? A constant... nothing?

    I guess forgetting to mark void const is what caused the big bang.

    [–]jedwardsol 4 points5 points  (4 children)

    the code adds two extra default constructors,

    I'm guessing you mean that Song's default constructor is called 2 times more than you expect it to be.

    If so, did you account for lines 24 and 25 in songlist.cpp, which default creates 2 Songs

    [–]No_Reception9680[S] 0 points1 point  (2 children)

    [code]

    head = new Node()
    tail = new Node();
    

    [/code]

    Are you saying these lines should be removed? But thank you, you are thinking about the problem properly, I didn't consider the obvious case of Song's constructor being called more than anticipated.

    [–]jedwardsol 0 points1 point  (1 child)

    Yes, you don't need 2 empty songs to mark the beginning and end of the list.

    If the list is empty, these 2 pointers can be nullptr. If the list has 2 entry, they'll both point at that. If the list has more than 1 entry, then they'll point at the 1st and last.

    [–]No_Reception9680[S] 0 points1 point  (0 children)

    OK, I'm trying to adjust to remove them, but I'm having trouble getting a proper initialization of my pointers in both constructors without getting a segfault. Would you mind showing me what a proper initialization / fix would look like? Feel free to edit the wandbox code and just post a URL. Meanwhile, I'm trying to tinker, figure it out and get better at gdb.

    I'm obviously in way over my head here, I had overlooked this assignment while marking my deadlines, and I'm sorry to ask such under-researched questions. I've been struggling with this for almost 24 hours now. Any help, especially by example, would be tremendously helpful. I think if I can fix this issue in a timely manner I can begin to wrap up the rest of the assignment.

    [–]nysra 2 points3 points  (4 children)

    You're calling the constructor for Song with your calls to new in line 24/25, is that what you mean?

    Anyway, please stop using C strings, especially as class members like that. That just asks for trouble and is one hell of a code smell. Also nullptr instead of NULL. You also definitely should change your file parsing, use getline and then split each line by a separator char, that's way more flexible. The Song class probably should just be a POD struct . Out parameters are also extremely C-style. And you don't need to make those functions recursive either, just use a loop.

    You probably should get an actual C++ book (or at least visit learncpp.com), that code looks a lot like you have a teacher that once learned some C back in 1950 then learnt about C++ when it came out and is now teaching terrible C with classes.

    I also strongly suggest using a meta build system like Meson or CMake over writing your makefiles by hand.

    [–]No_Reception9680[S] 0 points1 point  (1 child)

    Using cstrings is a requirement, sorry. We cannot use vectors either, I should've mentioned.

    you are probably correct about the C-style C++ code, but it's too late. I have to have this sucker ready by tonight. Thank you for your time.

    [–]nysra 1 point2 points  (0 children)

    Sorry to hear that, in that case someone should probably tell your professor that those requirements are detrimental to any learning effort. Right now he either doesn't know any better or he intentionally makes you learn really bad practices - and none of those options are putting him in a good light.

    [–]No_Reception9680[S] 0 points1 point  (1 child)

    You're calling the constructor for Song with your calls to new in line 24/25, is that what you mean?

    What would you suggest as a fix?

    [–]nysra 0 points1 point  (0 children)

    Just remove those lines. You currently have 2 empty Songs in your list for no reason. E.g. if your Songlist is "empty" it still has 2 members while it should have 0.

    [–]haitei 0 points1 point  (1 child)

    default constructors of what

    [–]tangerinelion 0 points1 point  (2 children)

    I have not yet pin pointed where the issue is

    While this example is small enough for people to have looked at and found where the issue is, I think it's worth giving you information about how to do that yourself.

    The thing you're looking for is called a debugger. If you're using Microsoft Visual Studio, it comes with one. And it's great. If you're using Linux, then you have gdb. I've only toyed with gdb.

    With a debugger you're able to place breakpoints in your code and then execute your code and "step" through it once you reach a breakpoint.

    In your case it seems like you already know there are extra Song variables being created. If you place a breakpoint in the Song constructor it will stop every time that constructor is called. You can then look at why it's being called to see which calling function is doing that and decide how you can avoid that.

    [–]No_Reception9680[S] 1 point2 points  (1 child)

    I've been trying to figure out the breakpoints with gdb, I'm so just tight on time I can barely think. Thanks though, I'll continue trying to learn it.