0

For example, suppose I have a class to create a button with specific styles common to my app:

class ButtonFactory{
public:
  static Button* createAppButton(std::string st,int font size){
    Button* b=new Button();
    //b->setColor();b->setText()...;b->playAnimation(...);
    return b;
  }
}

at other class

this->b=ButtonFactory::createAppButton("OK",20);
.
.
.
//delete it at destructor
delete b;

However, I feel uncomfortable that the 'new' and 'delete' are not at the same class. So my question is, should I also create a delete method in the ButtonFactory:

static void deleteAppButton(Button* b){
  delete b;
}
.
.
.
ButtonFactory::deleteAppButton(this->b);

instead of delete b by keywords directly?

  • The call to ButtonFactory.createAppButton is your creating statement. If you match that instead of new there is no issue. You should have a reason to use a factory though. In the example it does not look particularly useful. – Martin Maat Jul 25 '18 at 5:27
4
  • With manual resource management, this is a must. You definitely want to provide destructor method. C is a good illustration for this, literally every allocator has accompanying destructor.
  • With managed memory Button is supposed to implement Disposable interface, which manages destruction (compiler assisted). Java and C# are famous for their resource leaks caused by misuse of Disposables.
  • C++ and Rust (RAII resource management) suggest to return an object that manages it's own lifetime. RAII moves responsibility for resource management from a new owner to allocating code, separating ownership and resource management concerns.

In this case a method would probably return unique_ptr<Button>. Smart pointer could include a custom destructor, if default one is not suitable in future.

Other approaches may be justified, but there is nothing indicating that in your example. Please, explain, why do you use raw pointers?

  • shared_ptr<> would be a better fit here than unique_ptr<>, since its not unlikely the ptr would be needed in more than one place (due to how gui frameworks usually work - like list of buttons associated with a page, list of things with callbacks etc). – Lewis Pringle Jul 25 '18 at 13:35
  • @LewisPringle no, you have exactly one place which owns the std::unique_ptr<button>, and a bunch of other places that reference it ( button &) – Caleth Jul 25 '18 at 14:44
  • @LewisPringle unique_ptr is convertible to shared_ptr and does not have it's added cost of atomic reference counting. – Basilevs Jul 25 '18 at 15:23
  • @Caleth if you are going to use references, you may as well not be using smart pointers of any kind. The reference is an unsmart pointer. – Lewis Pringle Jul 25 '18 at 15:49
  • @LewisPringle when you can avoid using pointers of any kind, you definitely should. In OP's case pointers are only required due to legacy API. – Basilevs Jul 25 '18 at 15:51
0

The object of the class should be able to take care of its own cleanup. This is a good practice.

If you follow your approach with ButtonFactory::deleteAppButton(this->b), then the calling code has to know that the object of the class Button has to be disposed of in a special way. This increases the risk that somebody will forget to call the ButtonFactory::deleteAppButton(...).

Furthermore...

Imagine a Button class that can't take care of doing all of the destruction by itself, and needs assistance from another class. (For example, some resource for the Button objects come from a pool, and have to return to the pool. The pool management is done by a separate class.) Then the Button class should call the pool manager object in the destructor. To be able to do that, the Button class should keep a pointer to the pool manager object. The calling code should only do delete b.

0

This is very language-dependent.

What are you writing is good old wizdom of C coding. Because

  • C does not have any standard way of cleanup
  • C is used to produce highly portable shared libraries, and sometimes there is literally no "runtime" code shared by a library you use and rest of the application

So it is customary for C each API to have its own cleanup functions for own types, like curl_multi_cleanup()

For C++ situation is a bit different. There is a standard way to delete objects, moreover, you do not usually delete them explicitely, instead using RAII and destructors to do it. So for idiomatic C++ you should make object deletable with simple delete. Also, portability requirements for C++ code is much less strict - you usually expect that a library you use and code which uses it are build with same compiler and standard library and other dependencies.

  • What do you mean by less strict compatibility requirements? In absence of ABI nothing can be stricter, can it? – Basilevs Jul 25 '18 at 4:25
  • 1
    I meant not much more than there is already explained - that a C++ library is much less likely to be used in some very other application, built with different compiler, with unknown ways to manage its memory. ABI may save us from disaster if we try to do so, by failing linking, but it is not what is required. – max630 Jul 25 '18 at 5:46
0

The PURPOSE of a factory is to abstract away the choices of how to create a particular instance. This generally means, selecting what extra information to stick into it, and it subtype. If it implies some special handling of the deletion of the object, that should generally be injected into the smartpointer itself (in C++ deleter or Button or subclass destructor).

Factories have NOTHING whatsoever to do with object lifetime.

As other have mentioned, managing object lifetime is somewhat language dependent. Since you seem to be using C++ (and since IMHO that has the best/most power over this aspect of programming) - you would be well served to consider using smart pointers to manage the lifetime of your objects.

For example, I would use:

class ButtonFactory{
public:
  static shared_ptr<Button> createAppButton(std::string st,int font size){
    shared_ptr<Button>  = make_shared<Button> ();
    //b->setColor();b->setText()...;b->playAnimation(...);
    return b;
  }
}

Note - when you do this - its important to store an explicit copy of the smart pointer someplace, because when the last reference to the smart pointer goes away, the object will be deleted.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.