search

mucsci-students / 2019fa-420-Hercules

Team Hercules
0 stars 0 forks source link

S4 gui load node #127

Closed Qpirons closed 4 years ago

silviuburz commented 4 years ago

Looks good, but please create a function in the controller (UMLObjectHolder) that assigns X/Y for a specified UMLObject, since the view (GUI) should not be directly touching those. I thought I moved the GetUMLObject function to private, since it is not needed outside of the controller (and we already have proper controller functions to do everything)

Function should look like

//untested int SetX(std::string which_class, int x) { UMLObject * m = GetUMLObject (which_class); if (m == NULL) return ClassDoesntExist; m->SetX(x); return ElementSuccess; }

Qpirons commented 4 years ago

I really dont understand the need for that it seems needlessly complicated. We have the functions why cant we use them unless we create another function to access them?

Qpirons commented 4 years ago

What you asked for is done i still dont understand the reasoning for it but its done

bmekstro commented 4 years ago

On my phone so I can't test it, but the previous iteration worked fine storing X and Y coordinates for nodes. Maybe change the UMLObject *m to something else other than just m. Other than that it looks fine to me.

Qpirons commented 4 years ago

There i changed the name of the variable from m to node, to show that you are setting/getting a nodes x and y coordinates

silviuburz commented 4 years ago

Well done, looks great. Approved from me (should require 1 more person's approval too?)