New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Passing IDs instead of entire objects is not an Object-Oriented approach #10

Closed
manoelcampos opened this Issue Jun 10, 2016 · 1 comment

Comments

Projects
None yet
1 participant
@manoelcampos
Owner

manoelcampos commented Jun 10, 2016

IMPROVEMENT

The tool is a discrete event simulator that relies on message passing to execute simulation tasks.
However, several times it is passed raw data types into messages that are not type safe and makes
difficult to know what exactly a given method parameter has to receive.

Detailed information about how the feature should work

The SimEntity.send method, which is used by several sub-classes, receives an Object data parameter, that usually is an array of entities' IDs. The DatacenterSimple.processVmCreate method is an example of this problem, where the parameter is filled with the Datacenter ID, the VM id and a last integer to represent a boolean value.

Considering that the method send is used to pass several and totally different kind of data for different entities, it should be tried to pass a specific object with everything that is needed, instead of an int array. This object shouldn't just store these IDs into different attributes, but instead, store the objects that these IDs represent.

The problem of using IDs to create relationships happens in other contexts, such as to relate a Cloudlet to the Vm that will run it and the broker that represents the Cloudlet customer, it is used a set of IDs, respectively, vmId and brokerId, instead of a Vm and DatacenterBroker attributes.
The use of these IDs also requires Integers instead of entire objects when passing messages during simulation execution.

Some attributes that have to be changed from int to a specific object (the default value has to be set using the NULL object pattern)

  • Cloudlet.userId, Cloudlet.vmId
  • Vm.userId
  • VmScheduler and CloudletScheduler internal attributes
  • Objects of type List or Map where the key is an Integer representing the ID of some object instead of the object itself.
  • Several classes inside the network package, including all the NetworkPacket implementations.

A brief explanation of why you think this feature is useful

The proposal improves the OO design, makes the code type-safe and clear to understand and tries to avoid runtime cast exceptions.

Passing an entire object doesn't increase memory footprint once it is passed just a reference to an already instantiated object.

After all, creating true OO relationships using objects instead of IDs, allows you to navigate through the entities to get the data that you want. For instance, to know what is the Datacenter where a cloudlet is running would be possible just by calling cloudlet.getVm().getHost().getDatacenter().
This is true OO relationships and will allow developers using the simulator to get a lot of information about the execution of their simulations in an intuitive way.

@manoelcampos manoelcampos changed the title from Passing IDs instead of entire objects is not an Object-oriented way to do things to Passing IDs instead of entire objects is not an Object-Oriented way to do things Nov 17, 2016

@manoelcampos manoelcampos changed the title from Passing IDs instead of entire objects is not an Object-Oriented way to do things to Passing IDs instead of entire objects is not an Object-Oriented approach Nov 17, 2016

@manoelcampos manoelcampos self-assigned this Nov 17, 2016

@manoelcampos

This comment has been minimized.

Show comment
Hide comment
@manoelcampos

manoelcampos Dec 2, 2016

Owner

This improvement has been made along the time. The commit 954e32e also fixed the issue for Cloudlet, Vm, Host and Datacenter classes, allowing a chained call such as cloudlet.getVm().getHost().getDatacenter().

Owner

manoelcampos commented Dec 2, 2016

This improvement has been made along the time. The commit 954e32e also fixed the issue for Cloudlet, Vm, Host and Datacenter classes, allowing a chained call such as cloudlet.getVm().getHost().getDatacenter().

@manoelcampos manoelcampos added this to the CloudSim Plus 1.0 milestone Dec 14, 2016

manoelcampos added a commit that referenced this issue Dec 17, 2016

Closes #10
- Several classes were refactored to stop working with IDs and starting providing a true object oriented framework.
  Lots of attributes that were defined as a List of Integer or as a Map where the key was Integer, to store
  the ID of entities, were changed to store the object itself not its ID.
  It makes the framework far less error prone, since the Lists and Maps will just accept
  the defined object.
- It makes even clear to understand the framework code and to use it, since
  that when a developer look at some of such Lists or Maps, he/she will immediately know
  what such Lists/Maps are storing.
- Storing actual objects instead of their IDs also improves the framework performance
  since it doesn't have to get the object ID and look for the actual object inside
  another List that in fact stored the objects (as it was common to be done).
- Message passing mechanisms was also improved, storing actual objects into the SimEvent.data attribute
  instead of the object ID. The source and destionation of a message sent is also performed internally
  using just the ID, because changing these behaviour would require far more work.
- The documentation of such Lists/Maps was reviewed and updated.
- Network related classes were those ones that received lots of changes, mainly NetworkPacket implementing classes.

manoelcampos added a commit that referenced this issue Dec 17, 2016

Closes #10
- Several classes were refactored to stop working with IDs and starting providing a true object oriented framework.
  Lots of attributes that were defined as a List of Integer or as a Map where the key was Integer, to store
  the ID of entities, were changed to store the object itself not its ID.
  It makes the framework far less error prone, since the Lists and Maps will just accept
  the defined object.
- It makes even clear to understand the framework code and to use it, since
  that when a developer look at some of such Lists or Maps, he/she will immediately know
  what such Lists/Maps are storing.
- Storing actual objects instead of their IDs also improves the framework performance
  since it doesn't have to get the object ID and look for the actual object inside
  another List that in fact stored the objects (as it was common to be done).
- Message passing mechanisms was also improved, storing actual objects into the SimEvent.data attribute
  instead of the object ID. The source and destionation of a message sent is also performed internally
  using just the ID, because changing these behaviour would require far more work.
- The documentation of such Lists/Maps was reviewed and updated.
- Network related classes were those ones that received lots of changes, mainly NetworkPacket implementing classes.

@manoelcampos manoelcampos added this to the CloudSim Plus 1.0 milestone Dec 20, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment