Code not working as intended (everything is Kung Fu Panda 4)

[dry owl]

Why is every movie object the same?

c++

[short fjord]

When your question is answered use !solved to mark the question as resolved.

Remember to ask specific questions, provide necessary details, and reduce your question to its simplest form. For tips on how to ask a good question use !howto ask.

[dry owl]

none of the code really works as intended becuase of this, but once this issue is resolved i assume the rest will fix itself

[fallen summit]

i didn't go through it all, but in the case of listing movies, you may want to double check this function, there's a bug hiding in there.

void MovieTicketMaster::ViewShowingMovies()

a few other suggestions... why separate the Init() call from the instantiation of a MovieTicketMaster object? why not move that logic into the constructor, or just call Init() from the body of the constructor so it initializes itself automatically without an explicit call needing to be made separately?

you may also want to skip printing all of the blank entries in the movie_list when displaying the listing to clean up the output, that way it only outputs valid movie records.

and just an additional tip - if you haven't tried using a debugger yet, it will make issues like these very easy to track down and identify since you can just step through the code line by line, inspecting the value of any variable(s) in scope at any level of the call stack. if you haven't used one before and would like to try setting one up just tag me and I can help you out with that process.

also, i know it's common to see using namespace std; in homework assignments, and i'll never understand why professors like to encourage it's use, but it's good to avoid that line since outside of school assignments, you'll most likely never see it used in a codebase.

(that last bit has nothing to do with the bugs based on what i skimmed, but it's just good to get in the habit of avoiding using namespace std; in general)

[lilac comet]

Your main problem is that you are using for(auto movie: movie_list) to iterate, this creates copies of every movie during iteration, and loops over the empty movies from your previous question.

To fix it, that line in void MovieTicketMaster::ViewShowingMovies() should probably read for(int i = 0; i < movie_count; i++)

[dry owl]

I have Init as it’s own function and call it the way I did to follow the teachers guidelines in the assignment. But just so i know, what difference does calling it from the constructor do?

For the debugger i will definitely try that later (it’s 1am for me rn) you’re the second person that’s said that today so it’s probably a good idea lol.

[dry owl]

why tho doesn’t it make it easier to read?

[short fjord]

Why Is `using namespace std` Considered Bad Practice?

using namespace std will import all the symbols from std into the enclosing namespace. This can easily lead to name collisions, as the standard library is filled with common names: get, count, map, array, etc.

A key concern with using namespace std; is not what is imported now but rather what may suddenly be imported in the future.

While using namespace std; is alright for tiny projects, it is important to move away from it as soon as possible. Consider less intrusive alternatives:

// OK: *only* import std::vector
using std::vector;
// OK: namespace alias
namespace chr = std::chrono;
chr::duration x;

See Also

Why is "using namespace std;" considered bad practice?
Example of compile and runtime errors caused by it
Confusion Caused by "using namespace std"

[dry owl]

when would using auto appropriate then?

[fallen summit]

it's actually slightly different than what you described here, but since it's a homework assignment i didn't want to give it away. the copies are ok since the movie_list is just an array of pointers. there's something very subtle hiding in plain sight in there haha.

if you take a closer look i'm sure you'll spot it in a second

[dry owl]

ok thanks guys i’ll take a closer look in the morning

[fallen summit]

putting that call in the constructor will just call it automatically when you instantiate the object in main() like this:

MovieTicketMaster* p_movie_ticket_master = new MovieTicketMaster("Oakridge Theaters", "San Jose", 8);

if that Init() function is called in the constructor for MovieTicketMaster, that means it will be called whenever the object is created. if you code follows what the professor's guidelines look like, just stick with that instead though... normally all initialization logic would just be handled in the constructor.

(you also probably don't have to be newing this object unless that was also recommended by your assignment's intructions)

[dry owl]

ok ty

[lilac comet]

that movie_list isn't, it is defined as Movie movie_list[MOVIE_LIST_SIZE]; (at least in the my version, maybe it was updated)

[fallen summit]

i think it might have been updated if that's the case

[dry owl]

i think it said to dynamically make it, which would be using new correct?

[fallen summit]

this is what that function looks like for me @lilac comet ..

void MovieTicketMaster::ViewShowingMovies()
{
    Movie* p_movie = &movie_list[0];
    for (auto movie : movie_list)
    {
        p_movie->Display();
    }
}

[lilac comet]

Oh yeah, I forgot that I had fixed the issue you're talking about before (the one i'm talking about is that it's printing the destructor code every loop)

As a small hint to @dry owl, compare that loop to your other loops

[dry owl]

Alr ty guys i’ll make sure to check it out again when i wake up

[short fjord]

@dry owl Has your question been resolved? If so, type !solved :)

[fallen summit]

at second glance, what @lilac comet mentioned is actually correct in his first statement since it's not an array of pointers, it is in fact an array of Movie objects, so the copies are actually to blame with you seeing that destructor call text being printed. You clarifying that you're seeing the destructor print statement printing made that bit more obvious... but there's still a different bug in the function (the one I was referring to originally) that might make it all harder to make sense of since it's the cause of Kung Fu Panda 4 printing repeatedly. Just compare it to your other loops and that problem should go away when you make things consistent.

When you revisit this tomorrow, but don't understand why copying the objects is to blame when it comes to seeing that output from the destructor, just tag either of us and we can explain.

[dry owl]

@fallen summit @lilac comet i changed it to this and it shows all the different movies now, but why was the last for loop
void MovieTicketMaster::ViewShowingMovies()
{
Movie* p_movie = &movie_list[0];
for (auto movie : movie_list)
{
p_movie->Display();
}
}
not working?

oh is it cuz i never used movie lol?

[lilac comet]

Not quite, the problem was that you never moved the p_movie pointer. Or you would have had to use movie exclusively, and not p_movie in the loop

[dry owl]

ahh gotcha

thanks!

another question, i have now made move_list a dynamic array, and am attempting to sort it
what is the oversight in the if statements that im missing?

    Movie* temp= nullptr;
    for(int x = 0; x<kSize-1; x++){
        for(int y = 0; y<kSize-x-1;y++){
            if(p_movie_object->get_movie_name().compare((p_movie_object+1)->get_movie_name()) > 0)
            {
                temp =  movie_list[x+1];
                movie_list[x] = (p_movie_object+1);
                movie_list[x+1] = temp;
            }
        }
    }

[fallen summit]

i think you could have also done something like this as an alternative solution:

for (const auto& movie : movie_list)
    movie.Display();

[fallen summit]

what is move_list's type here? is it still a c-style array of Movie objects?

[dry owl]

its Movie* movie_list now

Here is updated code:

idk why its sending like that

[fallen summit]

and follow up question for you... do you understand how/why the Movie destructor's print message was being called when your loop was something like this:

for (auto movie : movie_list)
{
    //...
}

[dry owl]

every time it went to next movie object the last one just destroyed?

[fallen summit]

sort of, but not completely... have you ever heard of the term "RAII" (resource allocation is initialization)? it essentially describes the behavior of objects instantiated in C++ and what happens when the instantiated objects leave scope

https://en.cppreference.com/w/cpp/language/raii

[dry owl]

is it moving to the next object not the same as leaving scope?

[fallen summit]

when movie_list still contained Movie objects (not pointers like it does now), using auto was copying them each iteration of the loop.

[lilac comet]

To put it together, every time it got a movie, it made a copy for you to access, due to the loop effectively saying auto movie = movie[x]; (it's a little bit more complicated, but high-level), this movie went out of scope and was destroyed

[fallen summit]

so that copy of the object essentially lived only within the scope of a single loop iteration

[dry owl]

oh alright

[fallen summit]

so at the end of each loop iteration, the copy was destroyed, which calls the destructor

using auto& instead would reference the object from the list itself, avoiding the copy

[dry owl]

when would u use auto

rather then the normal datatype?

[fallen summit]

the & represents an object reference in case you haven't seen that before

[lilac comet]

auto is a bit of syntactic sugar mostly that makes it so that you don't have to type the whole datatype

[fallen summit]

yeah, usually just for primitive types or types that are faster to copy than to reference

[dry owl]

is it good practice to use auto rather than normal datatypes

[fallen summit]

eary on i'd probably avoid it's use unless the type is already clear on the same line of code

[dry owl]

ok

[fallen summit]

it's nice for iterator types since those are usually something like std::vector<some_type>::const_iterator and no one wants to type that out haha

[dry owl]

gotcha

for this it is a Movie* so i dont understand what the problem is

[fallen summit]

this snippet (in your older code) would just look like this for identical code (without using auto):

for (const Movie& movie : movie_list)
    movie.Display();

[fallen summit]

you're mixing Movie with Movie*

[lilac comet]

Movie * is effectively similar to Movie[], so the datatype of movie_list[x+1] is Movie (this is mostly due to some C inherited stuff)

[dry owl]

but its not Movie

[fallen summit]

yeah, the [] operator for arrays also acts as a pointer dereference

[lilac comet]

Essentially in C, (and your array is C-style, so it works the same), movie_list[x] is just syntactic sugar for *(movie_list + x)

[dry owl]

how would i re-reference

[lilac comet]

Also, can I implore you to use std::vector, rather than hand-rolling your own dynamic sized array?

[fallen summit]

is your assignment encouraging using c-style arrays btw? or are you allowed to use something like std::vector<Move*> or std::array<Move*, ArraySize> instead?

[dry owl]

we havent learned vectors yet

[lilac comet]

&movie_list[i + 1]

[fallen summit]

hah, you read my mind @lilac comet

[dry owl]

it wants us to turn what were arrays into now dynamic arrays

[fallen summit]

ok, so part of the assignment is to basically create your own dynamic array then?

[dry owl]

this is what it says exactly

i could of interpret it wrong

[fallen summit]

i'll never understand some of the homework assignments that get posted here lol. i guess this one isn't as bad as some others since it'll teach you some core concepts about how something like std::vector is implemented behind the scenes... sort-of haha, std::vector is obviously pretty complicated/optimized, but it'll give you an idea of the rough design when you have to build something that behaves similarly yourself

[dry owl]

so i understand what I'm doing here

temp = &movie_list[x+1]; //points temp to a reference of &movie_list[x+1] so now is =movie_list[x+1]

movie_list[x] = *(p_movie_object+1); //movie_list[x] is defreneces so i have to dereference the pointer to set it equal to eacother
movie_list[x+1] = *temp; // same thing here

[fallen summit]

i think you're interpreting it correctly

[dry owl]

this is our first assignment and im assuming vectors are prolly more complex than arrays for the most part

[fallen summit]

just to make sure you're not mixing terms up, temp is a pointer in your snippet above, not a reference. it's just pointing to the location of a Movie object that's been heap allocated (i'm assuming). the & can be used to get the address of an object (the way you're using it) which is essentially what a pointer would store. using & with a type (i.e. Movie& movie_ref = movie_list[i], would make it a reference.

pointers and references are conceptually similar, but are definitely not the same thing in C++.

[fallen summit]

yeah dynamic arrays / vectors are definitely more involved than a basic array. an array is a fixed size, but a vector can dynamically grow/shrink at runtime. the tricky part is that a dynamic array / std::vector should be implemented so the memory it's using allocates the objects contiguously in memory, but still has to support growing/shrinking as objects are added/removed from it.

[dry owl]

ah

[fallen summit]

everything else mentioned in your comments here is correct btw. to assign a value to one of the objects in the array, you'd need to dereference the pointer since movie_list[i] returns a Movie and not a Movie* (just like you're doing)

[dry owl]

tysm, my class is 100% asynchronous this helps alot

[short fjord]

@dry owl Has your question been resolved? If so, type !solved :)

[dry owl]

!solved

[short fjord]

Thank you and let us know if you have any more questions!

This thread is now set to auto-hide after an hour of inactivity

[fallen summit]

@dry owl - fyi i decided to take a look at what you had in the updated code you uploaded, but i think you have an issue in the init process with how you're handling movie_list... have you run into an issue with the code when you tried running it?

[dry owl]

Yea, the sort method just doesnt work and now if I try to buy a ticket to something not sold it says its sold out, Epic Tails also was now made with 0 available tickets

my code was working fine prior to this so its becuase of the dynamic array if i were to guess

[fallen summit]

yeah it definitely has something to do with the changes you made. in the last code you uploaded in the text file, it should be crashing before it ever even reaches the sort portion.

[dry owl]

my code isnt crashing tho, its just not workin as intended, unless crashing doesnt mean program stops

[fallen summit]

is your code identical to the code you uploaded last?

[dry owl]

example output:

it should be but ill re-send just incase

how do i send code w/o it turning to that^

wait some of that output is sold

old*

lemme give u just the new one

1 sec

im so confused

it works now

[fallen summit]

ok yeah you made some changes since the previous file you uploaded

[dry owl]

Seems only the sort isnt implemented correclty

or maybe is getting skipped

[fallen summit]

movie_list wasn't being allocated in the previous version you send (before the one just above)

[dry owl]

ah

[fallen summit]

and just to encourage you to look into setting up a debugger, this is the type of data you can view while you manually step through the code as it's running, even line by line if you want. that way you can easily diagnose what's going wrong in the sort code:

[dry owl]

i will thanks

[fallen summit]

a good debugger will quickly become your best friend once you get the hang of using one. they will turn hours of manual diagnosing issues using stuff like print statements into minutes of debugging when you can manually execute the program line by line and inspect any local variables while you step through the code.

which IDE are you using?

[dry owl]

vs code

[fallen summit]

what platform? windows? linux?

[dry owl]

windows

i think my problem may be that i never reset p_movie_object

[fallen summit]

so vscode is a bit tricky to set up compared to something like visual studio. if you're on windows i'd download that since it'll come preconfigured for you out of the box

there's even a setup guide to get you started here: https://discord.com/channels/331718482485837825/1165492293810257920

you basically just have to install it with the C++ toolchain selected during the install process, create a new console application project, then paste your code in and it should run directly from a debugger through the IDE with 0 additional setup/configuration. if you try it out and get stuck just tag me in a comment and I can show you the basics once you have it installed.

[dry owl]

ok ty

for the sorting i made a helper method and called it at the end of init (to solve the not resetting problem) but still seems to have no effect

[fallen summit]

also, to add a bit of what you learned above, involving how array[i] will automatically dereference the pointer at array + i, you can simplify some of your code in Init() like this:

    const int kSize = 8;
    const std::string_view movie_name_list[kSize] = {
        "Kung Fu Panda 4", "The First Omen",  "Monkey Man",
        "Migration",       "Arthur The King", "Godzilla X Kong: The New Empire",
        "Imaginary",       "Epic Tails",
    };

    const int seat_available_list[kSize]{
        4, 6, 13, 19, 2, 7, 12, 9,
    };

    const double price_list[kSize]{
        12.5, 11.3, 9.5, 4.0, 3.8, 10.0, 11.9, 14.5,
    };

    Movie* p_movie_object = movie_list;
    const std::string_view* p_movie_name_list = movie_name_list;
    const int* p_seat_available_list = seat_available_list;
    const double* p_price_list = price_list;

    for (int i = 0; i < kSize && i < MOVIE_LIST_SIZE; i++)
    {
        p_movie_object[i].set_movie_name(p_movie_name_list[i].data());
        p_movie_object[i].set_seats_avaiable(p_seat_available_list[i]);
        p_movie_object[i].set_ticket_price(p_price_list[i]);
    }

    for (int i = kSize; i < MOVIE_LIST_SIZE; i++)
    {
        // setting the rest of the indicies to empty objects
        movie_list[i].set_movie_name("");
        movie_list[i].set_seats_avaiable(0);
        movie_list[i].set_ticket_price(0.0);
    }

    movie_count = kSize;

[dry owl]

i wanted to do that cuz thats seemed easier but he speically said "Must use pointer syntax throughout when iterate through array elements or member functions of a Movie object. Array notation (such as movie_list[i]) or pointer notation such as ( (p_movie + i)->set_price ( ) ; ) is not allowed. Basically it's my wish for you to use the arrow notation ->. You may declare as many pointers as needed." :(

i assume that way you showed is the normal way

[fallen summit]

hah, typical professors. either way it's good practice to understand pointer syntax/arithmetic/iteration

[fallen summit]

just a hint about this... some of the logic in the sort function is slightly off

[dry owl]

so it is being called correcly just has logic errors

?

[fallen summit]

yeah, there's 2 different issues hiding in there

1. the comparison being made
2. the swap implementation

[dry owl]

im altering it rn

im getting closer

[dry owl]

void MovieTicketMaster::BubbleSort(){
const int kSize = movie_count;
Movie* p_movie_object = &movie_list[0];
Movie* temp= nullptr;
for(int x = 0; x<kSize-1; x++){
for(int y = 0; y<kSize-x-1;y++){
p_movie_object = &movie_list[y];
if(p_movie_object->get_movie_name().compare((p_movie_object+1)->get_movie_name()) > 0)
{
temp = &movie_list[y];
movie_list[y] = movie_list[y+1];
movie_list[y+1] = *temp;
}
}
}
}

No matter where i update the pointer it doesnt work, does that mean i have to do w/o or is there a way thast i an do with

[fallen summit]

you'll want to approach the swap without using pointers

[dry owl]

is there a way to do with pointers?

or is that too advanced fore my level

[lilac comet]

Your swap isn’t swapping the values, but saving one over the other and back - you need to make a temporary copy of the movie

As a hint, make temp a simple Movie, and go from there

[dry owl]

this works

but how do u do it the pointer way?

cuz at some point i assume i need to do it that way

[lilac comet]

You would have to have two levels of indirection, so you have a dynamic array of pointers to Movies - which are then created using new; though that is likely a battle for another day, along with your trusty debugger

Since things can go south fast when starting out with Movie**s

[dry owl]

all im hearing is a not this class problem 🤣