On Fri, 24 Feb 2017 08:26:30 +0100
Arvin Schnell
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)
To be honest I do not like both. I like to have ptable.type.gpt? or one level ptable.gpt?
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.
What is difference between specified exception and nil class exception? in both case it is programmer error and need to be fixed. If we do not want exception, it can be possible to Null object pattern and define behavior for non-existing filesystem, but
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.
As I write above, populating Storage namespace looks wrong. Enums on other hand is a bit previous century technique and used only very rare in ruby and looks a bit strange. So for both case I prefer to have disk.dasd? or disk.type.dasd? ( or what is sometimes used `disk.type == :dasd` but I think it is more error prone) and `disk.partitions.any?(&:dasd?)`
ciao Arvin
Just my 2c Josef -- To unsubscribe, e-mail: yast-devel+unsubscribe@opensuse.org To contact the owner, e-mail: yast-devel+owner@opensuse.org