• lysdexicOPM
      link
      fedilink
      English
      arrow-up
      1
      ·
      edit-2
      6 months ago

      Simply taking std::string by value (as it is a memory management class created for that explicit purpose) would have solved the problem without kneecapping every class you make.

      I think you are missing the whole point.

      The blogger tried to make a point about how special member functions can be tricky to get right if you don’t master them. In this case, the blogger even presents a concrete example of how the rule of 3/rule of 5 would fail to catch this issue. As the blogger was relying on the implicit special member functions to manage the life cycle of CheeseShop::clerkName and was oblivious to the possibility of copying values around, this resulted in the double free.

      You can argue that you wouldn’t have a problem if the string was used instead of a pointer to string, which is a point that the blogger indirectly does, but that would mean you’d be missing the root cause and missing the bigger picture, as you’d be trusting things to work by coincidence instead of actually knowing how they work.

      The blogger also does a piss-poor job in advocating to explicitly delete move constructors, as that suggests he learned nothing from the ordeal. A preferable lesson would be to a) not use raw pointers and instead try to adopt a smart pointer with the relevant semantics, b) actually provide custom implementations for copy/move constructors/assignment operators whenever doing anything non-trivial to manage resources, such as handling raw pointers and needing to both copy them and free them whenever they stop being used.

  • cmeerw
    link
    fedilink
    arrow-up
    9
    ·
    6 months ago

    And even the presented fix hurts my eyes. Should have used a unique_ptr or optional.

    • lysdexicOPM
      link
      fedilink
      English
      arrow-up
      1
      ·
      edit-2
      6 months ago

      You’d be missing the whole point too if you think that the pointer is the root cause. The problem is that the class is not designed to be copy-able, let alone moveable. Your suggestion to use a unique_ptr will also blow up in your face the moment you try to copy an instance.

      • cmeerw
        link
        fedilink
        arrow-up
        2
        ·
        6 months ago

        Depends on what semantic you want. Sure, if you use a unique_ptr member, you will get a deleted copy constructor/operator - I wouldn’t consider that blowing up in my face.

    • lysdexicOPM
      link
      fedilink
      English
      arrow-up
      1
      arrow-down
      1
      ·
      6 months ago

      Naked pointers are just too stupid for modern C++ ;)

      Anyone who works on real-world production software written in C++ knows for a fact that pointers are a reality.

      Also, there are plenty of frameworks who employ their own memory management frameworks, and raw pointers are perfectly fine in that context. For example, Qt uses raw pointers extensively because It’s object system implements an object ownership system where each object can have child and parents, and you can simply invoke deleteLater() to free the whole dependency tree when you no longer need it.

      • vzq@lemmy.blahaj.zone
        link
        fedilink
        arrow-up
        3
        arrow-down
        1
        ·
        edit-2
        6 months ago

        Your reply is an amazing mix of condescension and not getting the point.

        I’m having it framed.

  • CameronDev
    link
    fedilink
    arrow-up
    4
    ·
    6 months ago

    I got stuck on cheese counting:

    inventory.count(cheeseName) == 1 ? "Yes" : "No"
    

    1 cheese == some, 2 cheese == none :/

    • rasensprenger@feddit.de
      link
      fedilink
      arrow-up
      3
      arrow-down
      1
      ·
      6 months ago

      Yeah, that’s confusing to read. A hashset can only contain at most one of each cheese kind, in c++20 you can write inventory.contains(cheeseName), before that you had to use .count or inventory.find(cheeseName) != inventory.end() or something like that