On Tue, 24 Nov 2015 15:47:24 +0100
Stefan Hundhammer
On 24.11.2015 15:15, Josef Reidinger wrote:
Well, in OOP is also one more approach called NullObject pattern, which is create kind if partition table, that in represent no partition table.
This is wrong on so many levels that I don't even know where to begin...
I know it is a bit controversy, similar like Singleton pattern. I agree that over use it is bad design, but I also think it works nice in some situations.
This proposed NullObject pretends to do a lot of things that it can't deliver.
If something cannot be deliver then it is fair to raise exception in this method. It is common used approach. Where it make sense provide functionality and where it cannot deliver it, then raise exception.
It makes false promises.
promise depends on API contract.
And I can't even tell that it does that - to the outside world, it looks just like the real thing; the difference being that it does not do anything useful, so there is no reasonable way to handle the other case.
Comparing to `if not nil` or `catch Exception` I think it can do similar stuff like logging. Of course as I mention above that method can also raise exception if something is required that cannot be delivered.
so if NoPartitionTable is used and its partitions return single partition, that is in fact filesystem on disk.
Only that I really need to know the difference in most cases because I have to do different things. To stay with the current example: A disk might have
- an old-style MS-DOS type partition table with 4 slots (where I need to use an extended partition if I need any more partitions)
- a new style GPT partition table that can have any number of parititions
- a filesystem directly on the disk device without any partition table
So for me I can see it as three types of partition table - msdos, gpt and no. I do not see why is better to have begin if gpt? elsif msdos? end rescue # no partition table case end then simple if gpt? elsif msdos? elsif no? end
It sounds tempting to hide those gory details in the lib. But then, we do need to know a lot about this because we might have to do different things in the storage or bootloader proposal.
I do not see how how it hides details. It should be accessible for ones who is interested in it, but hide to ones who is not interested. Like in example above, if you need behave differently then detect it, but if you want find boot disk, you are not interested what kind of partition table is used.
It might just change the requirements if or when we need any of the boot partitions (/boot, EFI-boot, PReP) etc. etc.
So, since we are doing deeply technical stuff here, I fear we need to know and to handle the gory details. Too much information hiding will become counterproductive here pretty quickly.
For me it is not hiding, it just using specialized object or class as nil. Object that provide basic functionality or behavior for some cases and raise exception in others. Class that do not hide it is null one if you need to know it. so how can looks implementation for NoPartitionTable ( let me use ruby pseudocode to make it shorter ) class NoPartitionTable def create_partition(name, type) raise UnsupportedOperation, "Cannot create partition for partitionless disk" end def delete_partition(name) # act same as other classes when trying to delete non existing name end def get_partitions # return list with single element containing volume as partition end def get_partition(name) # act same as other classes when trying to get non existing name end def get_unused_partition_slot(all = true, logical = true) return [] end end and thats it. So no fake promises, no hidding details and it can be used in code interested in it. And advantage is also that usage is same as for other partition tables.
There be dragons. ;-)
JFYI new ruby 2.3 will have &. operator for such cases, so you can then write it like:
if disk&.partition_table&.gpt?
Yikes, that's really ugly.
I also more like try syntax from active support, but at least I see some logic in that syntax.
When Moses came down mount Sinai, he must have dropped some stone slabs - that one, for example:
Thou shalt not use crazy interpunction in the middle of source code.
;-)
which looks a bit better
I strongly disagree on that point.
probably matter of taste, I am too lazy to type too much characters :)
OTOH if there would be more checks that might fail (such as the 'if disk && disk.partition_table && disk.partition_table.gpt?' check above), that one 'begin..rescue' block would handle them all, and the control flow remains clear and straightforward.
And that's the beauty of it.
Well, I am not sure as you should use more specialized exception catcher, otherwise you can catch also other problem, that should go more top level, so in the end, you might want to rescue from three kinds of exception like
rescue NoPartitionTable, NoDisk, NotGpt => e
Those things tend to accumulate - the exception types as well as the ones caught in the rescue clauses.
It is questionable. If partitionless disks will be more common, then more code must cope with it and it makes more methods that have to be aware of it. So in the end it maybe not accumulate so much. That is reason why exception should be really exceptional case, which other code is usually not interested in.
Using exceptions and not checks all over the place can help to clarify the algorithm without clobbering the code with that checking code - code that is very rarely executed and thus mostly untested (try to create test cases for all those if/else branches!).
on other hand testing all potential raiser of exception is also quite tricky.
There are much less cases to test.
I worry that in more complex or top level code it can end that any line can raise multiple exceptions, which can make tests hard, but I do not have experience with using exceptions for this purpose.
Kind regards
Josef -- To unsubscribe, e-mail: yast-devel+unsubscribe@opensuse.org To contact the owner, e-mail: yast-devel+owner@opensuse.org