On Tue, Oct 13, 2015 at 01:05:16PM +0200, Josef Reidinger wrote:
> On Tue, 13 Oct 2015 11:29:44 +0200
> Arvin Schnell <[email protected]> wrote:
> 
> > On Tue, Oct 13, 2015 at 10:14:03AM +0200, Josef Reidinger wrote:
> > 
> > > > I'll do more experiments with this as soon as the prototype is
> > > > easily installable. But looks pretty clear that it would be
> > > > fairly easy to fix the un-rubism by adding the corresponding
> > > > methods in the Ruby side in case we decide we are so purist that
> > > > we cannot live with the current API. :-) So nothing to fix in the
> > > > libstorage (C++) side.
> > > 
> > > I agree, that it make sense to have some helpers to have ruby
> > > bindings more "ruby".
> > 
> > Just remember that those need documentation and test cases.
> > 
> > > > >> a) Is device_graph always an object representing the whole
> > > > >> graph?
> > > > > 
> > > > > The devicegraph always represents the whole graph.
> > > 
> > > 
> > > Well, in general I do not like this API before and I also do not
> > > like it now :)
> > > It is not object oriented and whats more device_graph is god like
> > > object.
> > 
> > Please elaborate this. What do you mean by God like object? A
> > object that holds all storage objects?
> 
> Yes, if you have object that hold everything it is not graph, but flat
> structure.

Sure it's a graph, see
https://en.wikipedia.org/wiki/Graph_%28mathematics%29, esp. "a
graph is a [...] pair G = (V, E) comprising a set V of
vertices [...] together with a set E of edges [...]".

> Regarding god objest see
> https://en.wikipedia.org/wiki/God_object it is one extrem, where you
> have too powerfull object.

I consider a single search function, as was proposed, as too
powerful (among other problems already explained).

> > Please explain that, esp. what problems you see with the existing
> > target map and how you still see this problems with the new
> > design.
> 
> Problem of target map is that it is not much flexible. It is not
> object, it is more like data. And if data changed, then all methods
> that use it need to adapt. In general rule for design is
> 
> 1) if data is fixed and just way how it is interpreted changed, then
> create data object and have methods that work on top of it.
> 
> 2) if data is changed, but way how it is worked with them do not
> change, then use object that represent such data.
> 
> Reason is easy changes. If you need to add new data type, is it easier
> to change device_graph and all methods that do finding or use some kind
> of child to represent it and connect it to rest of system?

These general rules don't help much when we discuss concrete
examples.

> > 
> > > I prefer to have graph of objects that points to each other
> > 
> > The device-graph is just that and the functions to query the
> > "pointers" exist.
> 
> That is something different. Now you have one object that holds it. so
> it is like
> 
> device_graph -> A, B, C, D, E

That is not a graph but just a list of nodes without edges.

> if it use graph of object then it can look like
> 
> device_graph -> filesystems -> A, B
>              -> physical_devices -> C, D
>              -> containers -> E
> and what is more important E point to C, D....C can point to partition
> C1, C2 and C1 can point to A and C2 can point to B.

In a graph everything can point to everything. The "pointer" can
even hold data.

> This way if something need disk, it can query it for partitions and
> inspect its filesystems without knowing if it is teoretic device graph,
> real one or modified one.

In the current prototype the functions can work on all graphs,
whether it's the "system", the "staging" or "my ideas" graph.

> > > and it is easy to say what object it returns. It also allows
> > > easy introspection and better documentation that allows easier
> > > usability.
> > 
> > In general your remarks are to buzz-work like to comment on them.
> 
> OK, let me explain it better. and use ruby as example.
> 
> If I am interested what e.g. device_graph provide me, I can do 
> device_graph.methods which in case passing device_graph everywhere do
> not should interesting stuff. Also methods is not documented in it and
> lives elsewhere.

Sorry, I don't understand that.

> In case when you use object like approach then you can do something like
> 
> disk.methods -> partitions, label, ...
> 
> and then you can check what provide you partitions, what provide you
> label, etc.

If I get you right you want to replace documentation by
introspecion.

> So it is easier to understand whole picture as it is layer knowledge.
> You have reasonable small Disk object and if it return in some method
> e.f. Partition object, then again you can check it and see what it
> provides if you need it.

The documentation will include a tree of storage classes so you
can get that information very easy.

> Counter example now is Storage module in yast2-storage which is
> overloaded by methods. Current approach in new libstorage for me looks
> like we have all methods for device_graph which is just enclosed in
> namespaces like 
> Storage::Filesystem.find_by_mountpoint(device_graph, "/")
> 
> which for me is just something like
> device_graph.find_filesystem_by_mountpoint("/")

Then you don't understand the basic prinicipe of OOP, that the
methods are part of the class that hold the data.

> > In general agreed, but if you have a get_xx and a set_xx function
> > a find_by_xx seems natural.
> 
> That depends how often it will be extended.

It should not happen so often. And you will have to add the
getter and setter anyway.

ciao Arvin

-- 
Arvin Schnell, <[email protected]>
Senior Software Engineer, Research & Development
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 
(AG Nürnberg)
Maxfeldstraße 5
90409 Nürnberg
Germany
-- 
To unsubscribe, e-mail: [email protected]
To contact the owner, e-mail: [email protected]

Reply via email to