[openbeosstorage] Re: BDiskSystem::Supports*()/Validate*()
- From: "Axel Dörfler" <axeld@xxxxxxxxxxxxxxxx>
- To: openbeosstorage@xxxxxxxxxxxxx
- Date: Sun, 27 Jul 2003 11:09:18 +0200 CEST
"Ingo Weinhold" <bonefish@xxxxxxxxxxxxxxx> wrote:
[...]
> > rarely needed, I think it would be a good idea to have the
> > ContentSize() feature in there if we're going to support
> > non-destructive enlarging even when the contents of the partition
> > cannot (and are not) enlarged.
> OK, it's no problem to add it. The question is then, whether we want
> to
> allow resizing the contents individually
> (BPartition::ResizeContents(off_t)), or at least allow to pass both
> size and content size to Resize():
>
> status_t Resize(off_t size, off_t contentSize);
>
> CanResize() and ValidateResize() would need to be adjusted
> accordingly.
> Against separating the features speaks, that it's more work for the
> API
> user, since they must take care of the order of applying the resize
> operations (contents, partition when shrinking, the other way around
> when enlarging).
I think it would be okay if contentSize had a default parameter in
which case it'll be the same as size. OTOH I don't think those changes
are worth the effort, since I cannot really imagine a situation where
you want to do that.
> > > True enough, but it would always create the need to have another
> > > variable and check, even if what I want to know would be this:
> > > bool canResizeContents;
> > > if (CanResize(&canResizeContents) && canResizeContents) {
> > > ...
> > > }
> > >
> > > I think it would be clearer to write:
> > > if ((ResizeCapabilities() & (CAN_RESIZE | CAN_RESIZE_CONTENTS)) !
> > > =
> > > 0)
> > > {
> > > ...
> > > }
> Unfortunately not having the same semantics though. :-P
> It would correctly read:
> if (!(~ResizeCapabilities() & (CAN_RESIZE | CAN_RESIZE_CONTENTS))) {
> ...
> }
> (significantly longer constant names BTW ;-)
>
> Looking less sexy I have to admit.
Oh, yes, I see.
> > > But if the contents of the partition are supposed to be going
> > > away
> > > anyway, you could also just check:
> > > if (CanResize()) {
> > > ...
> > > }
> > > without having to define an extra boolean parameter.
> That would work with the current version as well, when providing a
> default parameter for `canResizeContents'. I didn't, because I
> assumed
> that one would always be interested in the full truth, but it's no
> big
> deal to change that.
Perhaps we should do that, then?
> > > OTOH for DriveSetup its use would probably nice, I have to admit:
> > >
> > > bool canResizeContents;
> > > if (CanResize(&canResizeContents)) {
> > > if (!canResizeContents)
> > > warnUserAndAbortIfHeWantsTo();
> > >
> > > // resize the partition
> > > ...
> > > }
> Yep.
Since DriveSetup is the main idea behind this API, maybe we just ditch
my ResizeCapabilities() idea again, and keep what we have now?
> > > True, CanResize() wouldn't be needed anymore - yet the value it
> > > returns is as confusing as in the current solution (IMO).
> > If we don't need CanResize(), then why not ditch it? The only
> > problem
> > I can see with ResizeCapabilities is that is doesn't match the
> > CanXYZ()
> > naming scheme.
> Yes, that's pretty much what I think too.
I could also undo my suggestions, and we stay with what we have - since
it seems well suited for the use in DriveSetup and similar apps.
> > I'm not sure I see what would be gained by splitting CanMove() into
> > two
> > functions, though. If you really want to be able to get just the
> > info
> > returned by CanMove(), why not allow NULL to be passed for the
> > BObjectLists and provide default parameters?
> Same here, no big deal to do that.
Yes, that would be nice.
Adios...
Axel.
Other related posts:
