Conversation
kks32
left a comment
kks32
left a comment
There was a problem hiding this comment.
Avoid explicitly saying get_ and set_ methods, use function overlaoding instead. Please revise the class as requested
include/agent.h
Outdated
| //! \param[in] agent_id Agent id assigned to the agent object | ||
| void set_id(graph::vertex_t agent_id) { this->agent_id_ = agent_id; } | ||
| //! Get id | ||
| graph::vertex_t get_id() const { return agent_id_; } |
There was a problem hiding this comment.
Please don't use get and set methods, instead overload these functions just use id
There was a problem hiding this comment.
Do not understand... Do you mean I should define two functions both named id, one has inputs (to set the id) while the other does not (to get the id)?
include/agent.h
Outdated
| int get_status() const { return status_; } | ||
|
|
||
| //! Compute path based on current node and destination | ||
| void compute_agent_path(); |
There was a problem hiding this comment.
I'd suggest we assign path to agent, rather than compute it at agent level.
include/agent.h
Outdated
|
|
||
| //! Set origin and initialize current node to origin | ||
| //! \param[in] origin Origin vertex assigned to agent | ||
| void set_origin(graph::vertex_t origin) { |
There was a problem hiding this comment.
This is not the constructor, this is just a function.
kks32
left a comment
kks32
left a comment
There was a problem hiding this comment.
Why is there a .clang-format file?
include/agent.h
Outdated
|
|
||
| //! Set origin and initialize current node to origin | ||
| //! \param[in] origin Origin vertex assigned to agent | ||
| void set_origin(graph::vertex_t origin) { |
There was a problem hiding this comment.
We can have one function to set origin and destinations
| graph::vertex_t destination_{std::numeric_limits<graph::vertex_t>::max()}; | ||
|
|
||
| //! Agent departure time | ||
| graph::weight_t departure_time_{std::numeric_limits<graph::weight_t>::max()}; |
Codecov Report
@@ Coverage Diff @@
## develop #10 +/- ##
===========================================
+ Coverage 93.98% 94.17% +0.19%
===========================================
Files 11 13 +2
Lines 548 566 +18
===========================================
+ Hits 515 533 +18
Misses 33 33
Continue to review full report at Codecov.
|
| //! \param[in] id Agent id | ||
| //! \param[in] origin Agent origin | ||
| //! \param[in] destination Agent destination | ||
| explicit Agent(const graph::vertex_t& id) : id_{id} {}; |
There was a problem hiding this comment.
Please only declare functions in the header, do not define them. Please define functions in the implementation .cc file
There was a problem hiding this comment.
But I see { return nvertices_; } in graph.h
include/agent.h
Outdated
| graph::vertex_t current_node() const { return current_node_; } | ||
|
|
||
| //! status: 0: haven't started routing. 1: en_route. 2: arrived. | ||
| enum class Status { PREDEPART, ENROUTE, ARRIVE }; |
There was a problem hiding this comment.
This should be defined outside the Agent class.
| enum class Status { PREDEPART, ENROUTE, ARRIVE }; | |
| enum class Status { YETTODEPART, ENROUTE, ARRIVED }; |
There was a problem hiding this comment.
Why? Would they also be used by other classes?
| @@ -0,0 +1,66 @@ | |||
| // #include <vector> | |||
There was a problem hiding this comment.
Remove these if you are not using these
|
|
||
| //! Return agent status | ||
| //! \retval Status of the agent. | ||
| //! destination. |
There was a problem hiding this comment.
Why does it say destination?
3 participants
Create an agent class #7
Getting current edge not included yet.