On Fri, Feb 24, 2017 at 02:43:32AM +0100, Ancor Gonzalez Sosa wrote:
> On 02/23/2017 09:31 PM, Arvin Schnell wrote:
> > On Thu, Feb 23, 2017 at 05:45:48PM +0100, Ancor Gonzalez Sosa wrote:
> > puts "Ptable type #{a_storage_msdos_ptable.type}" #=> Prints "Ptable type
> > 1"
> >
> > puts "Ptable type #{a_storage_msdos_ptable.to_s}"
>
> Ok, so let's use another example.
>
> puts "Mount by #{blk_fs.mount_by}" #=> Prints "Mount by 2"
>
> This time, we already used Filesystem#to_s to display the filesystem
> type. So we cannot apply the same workaround to this other enum-typed
> field in the same class.
>
> As I wrote in the PR description, for some cases we have workarounds and
> for others don't. Abusing the #to_s method of Filesystem or
> PartitionTable to get the readable version of its type is just a
> workaround for one of the many drawbacks explained for whole enums
> approach. It does not address the root issues.
For all enums there is a function to turn the value into a
string. If desired the get_classname function can also be made
public, although I don't see a use-case (internally it is only
used at a very few places).
> > or
> >
> > puts "extended" if [Storage::FsType_EXT2, Storage::FsType_EXT3,
> > Storage::FsType_EXT4].include?(a_storage_filesystem)
> >
> > puts "extended" if Storage.ext?(a_storage_filesystem)
> >
> > Ok, ext? is new (part of my hackweed project). So far ext2 and
> > ext3 were not implemented.
> >
> > ptable.type == Storage::PtType_GPT
> >
> > Storage.gpt?(ptable)
>
> Once again, all those methods mixed altogether directly on the Storage
> namespace instead of being the responsibility of more focused classes
> look like a workaround to me. Not to mention the risk of name collision
> if all the values of all the possible enums need to have a corresponding
> method in the same place.
It doesn't look like a workaround for me - but we had the
discussion already a few years ago. And with strictly types enums
collisions are not a problem (and they are used in libstorage-ng).
> > Concerning the nil instead of exceptions:
> >
> > a_y2storage_unused_partition.filesystem #=> nil
> >
> > What happens here (nilclass exception?)?
> >
> > a_y2storage_unused_partition.filesystem.mount_by?
>
> If you call NilClass#mount_by? you will get an exception, of course. Not
> sure what's the point of the question, though.
Well, you know how many bug-reports we had with 'undefined method
X for nilclass'? To me that shows the downside of using nil.
> > I also thought about placing some classes in sub-namespaces but
> > mainly to resolve the upcoming name collision of DASD (the
> > "disk") and DASD (the partition table).
>
> In addition to the class names, this serves as a nice example of my
> point above. If at some point we have DASD as a possible value for a
> PartitionType enum and as a possible value for a, let's say, Transport
> or DiskType enum. Will the Storage.dasd? method be reused for both?
> Quite tricky.
Again, with strictly types enums that is not a problem.
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]
