Mailinglist Archive: yast-devel (87 mails)

< Previous Next >
Re: [yast-devel] Feedback needed! Hack Week project to re-define storage-ng API
  • From: Josef Reidinger <jreidinger@xxxxxxx>
  • Date: Fri, 24 Feb 2017 09:28:24 +0100
  • Message-id: <20170224092824.1449bbc6@linux-vvcf.privatesite>
On Fri, 24 Feb 2017 08:26:30 +0100
Arvin Schnell <aschnell@xxxxxxxx> wrote:

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@xxxxxxxxxxxx
To contact the owner, e-mail: yast-devel+owner@xxxxxxxxxxxx

< Previous Next >
Follow Ups