Code review - Issue 39820045: energy module now is accessible with Config pathhttps://codereview.appspot.com/2014-01-12T09:50:32+00:00rietveld
Message from unknown
2014-01-03T17:07:07+00:00orazio.brianteurn:md5:c0c024ca776dada02fafa96020373809
Message from tommypec@gmail.com
2014-01-11T18:47:22+00:00Tommaso Pecorellaurn:md5:7cbe9cd50912c7a8a12ba5bfd558297e
Hi,
beside the comments, can you provide a rationale of the changes ? I know it should be obvious, but still…
Cheers,
T.
https://codereview.appspot.com/39820045/diff/1/src/energy/helper/energy-source-container.cc
File src/energy/helper/energy-source-container.cc (right):
https://codereview.appspot.com/39820045/diff/1/src/energy/helper/energy-source-container.cc#newcode31
src/energy/helper/energy-source-container.cc:31: TypeId
please use uncrustify on the source code: utils/check-style.py will help you with it.
https://codereview.appspot.com/39820045/diff/1/src/energy/model/device-energy-model-container.cc
File src/energy/model/device-energy-model-container.cc (right):
https://codereview.appspot.com/39820045/diff/1/src/energy/model/device-energy-model-container.cc#newcode38
src/energy/model/device-energy-model-container.cc:38: .AddAttribute("List","The DeviceEnergyModels associated to the Node",
uncrustify...
https://codereview.appspot.com/39820045/diff/1/src/energy/model/device-energy-model-container.h
File src/energy/model/device-energy-model-container.h (right):
https://codereview.appspot.com/39820045/diff/1/src/energy/model/device-energy-model-container.h#newcode54
src/energy/model/device-energy-model-container.h:54: /*
Doxygen !
https://codereview.appspot.com/39820045/diff/1/src/energy/model/energy-source.h
File src/energy/model/energy-source.h (right):
https://codereview.appspot.com/39820045/diff/1/src/energy/model/energy-source.h#newcode180
src/energy/model/energy-source.h:180: std::vector<Ptr<DeviceEnergyModel> > m_models;
Why this has been changed ?
Message from unknown
2014-01-11T20:38:22+00:00orazio.brianteurn:md5:0e621ab9f9a0a3e5f0ba67668022b512
Message from orazio.1985@gmail.com
2014-01-11T20:42:05+00:00orazio.brianteurn:md5:70c3dbe9d17a077d28919e14f188d5ad
I fixed files.
I use std::vector<Ptr<DeviceEnergyModel> > instead of DeviceEnergyModelContainer because the last one have Begin(), End(), Iterator() and not begin(), end(), const_iterator. (required by object-vector)
On 2014/01/11 18:47:22, Tommaso Pecorella wrote:
> Hi,
>
> beside the comments, can you provide a rationale of the changes ? I know it
> should be obvious, but still…
>
> Cheers,
>
> T.
>
> https://codereview.appspot.com/39820045/diff/1/src/energy/helper/energy-source-container.cc
> File src/energy/helper/energy-source-container.cc (right):
>
> https://codereview.appspot.com/39820045/diff/1/src/energy/helper/energy-source-container.cc#newcode31
> src/energy/helper/energy-source-container.cc:31: TypeId
> please use uncrustify on the source code: utils/check-style.py will help you
> with it.
>
> https://codereview.appspot.com/39820045/diff/1/src/energy/model/device-energy-model-container.cc
> File src/energy/model/device-energy-model-container.cc (right):
>
> https://codereview.appspot.com/39820045/diff/1/src/energy/model/device-energy-model-container.cc#newcode38
> src/energy/model/device-energy-model-container.cc:38: .AddAttribute("List","The
> DeviceEnergyModels associated to the Node",
> uncrustify...
>
> https://codereview.appspot.com/39820045/diff/1/src/energy/model/device-energy-model-container.h
> File src/energy/model/device-energy-model-container.h (right):
>
> https://codereview.appspot.com/39820045/diff/1/src/energy/model/device-energy-model-container.h#newcode54
> src/energy/model/device-energy-model-container.h:54: /*
> Doxygen !
>
> https://codereview.appspot.com/39820045/diff/1/src/energy/model/energy-source.h
> File src/energy/model/energy-source.h (right):
>
> https://codereview.appspot.com/39820045/diff/1/src/energy/model/energy-source.h#newcode180
> src/energy/model/energy-source.h:180: std::vector<Ptr<DeviceEnergyModel> >
> m_models;
> Why this has been changed ?
Message from tommypec@gmail.com
2014-01-11T21:39:59+00:00Tommaso Pecorellaurn:md5:ff63fbd5677bbac0bc0f2576c6fd5801
I see… this makes me wonder why we d have a DeviceEnergyModelContainer as a class and not as a typedef.
The very same functionality could have been done with a simple:
typedef std::vector< Ptr<DeviceEnergyModel> > DeviceEnergyModelContainer;
well, more or less.
Sadly, removing DeviceEnergyModelContainer could be a major issue.
Said so, I have no more comments.
https://codereview.appspot.com/39820045/diff/20001/src/energy/helper/energy-source-container.cc
File src/energy/helper/energy-source-container.cc (right):
https://codereview.appspot.com/39820045/diff/20001/src/energy/helper/energy-source-container.cc#newcode37
src/energy/helper/energy-source-container.cc:37: .AddAttribute ("List", "The EnergySources associated to the node",
Change this attribute name to something more explicative (sorry, I forgot in the first revision).
Message from unknown
2014-01-12T09:46:58+00:00orazio.brianteurn:md5:eb8d7b7314d94ca458c5a7c455e3fcb1
Message from orazio.1985@gmail.com
2014-01-12T09:50:32+00:00orazio.brianteurn:md5:a7770b099d74def8f06f9c0697f4812c
I know what you mean... but this module is used in other modules and i don't want make radical changes...
Best Regards.
On 2014/01/11 21:39:59, Tommaso Pecorella wrote:
> I see… this makes me wonder why we d have a DeviceEnergyModelContainer as a
> class and not as a typedef.
>
> The very same functionality could have been done with a simple:
> typedef std::vector< Ptr<DeviceEnergyModel> > DeviceEnergyModelContainer;
> well, more or less.
>
> Sadly, removing DeviceEnergyModelContainer could be a major issue.
>
> Said so, I have no more comments.
>
> https://codereview.appspot.com/39820045/diff/20001/src/energy/helper/energy-source-container.cc
> File src/energy/helper/energy-source-container.cc (right):
>
> https://codereview.appspot.com/39820045/diff/20001/src/energy/helper/energy-source-container.cc#newcode37
> src/energy/helper/energy-source-container.cc:37: .AddAttribute ("List", "The
> EnergySources associated to the node",
> Change this attribute name to something more explicative (sorry, I forgot in the
> first revision).