1
    struct co_ordinate {
        double x;
        double y;
    };

// -----> DESIGN 1

    class Land {
      public:
        Land(std::vector<co_ordinate> co) shape_before_plotting(co) {}
        // actions
        // creates stage 2
        // populates shape_after_plotting_outside_boundary
        void do_plotting_of_boundary(size_t off_set){ 
              // logic here
              // this off_set is used to know where to add new co_ordinates
              // in the boundary
              // (the beginning and end of boundary is untouched)      
       }
        // creates stage 3
        // populates shape_after_plotting_land
        void do_internal_plotting(){
        // logic here
        //  adds new co-ordinate where line intersected inside the land after
        // stage 2.
        }

        // getters
        std::vector<co_ordinate> get_shape_before_plotting{
        return shape_before_plotting;
       }

        std::vector<co_ordinate> get_shape_after_plotting_outside_boundary {
        return shape_after_plotting_outside_boundary;
        }

        std::vector<co_ordinate> get_shape_after_plotting_land {
        return shape_after_plotting_land;
        }

       private:
        std::vector<co_ordinate> shape_before_plotting;

        std::vector<co_ordinate> shape_after_plotting_outside_boundary;

        std::vector<co_ordinate> shape_after_plotting_land;
    };


   int main(){
      Land my_land(vec);
      // stage 2 populates shape_after_plotting_outside_boundary
      my_land.do_plotting_of_boundary();

      // stage 3 populates shape_after_plotting_land 
      my_land.do_internal_plotting();

      // get co ordinate here

    }

// -----> DESIGN 2

    class Land {
     public:
        Land(std::vector<co_ordinate> co) plotting(co) {}

        Land operator=(const Land& other) {// logic for assignment operator}

          // actions

         // creates stage 2
         // populates shape_after_plotting_outside_boundary
        void do_plotting_of_boundary(size_t off_set){ 
              // logic here
              // this off_set is use know where to add new co_ordinates
              // in the boundary
              // the beginning and end of boundary is untouched 

       }
        // creates stage 3
        // populates shape_after_plotting_land
        void do_internal_plotting(){
        // logic here
        //  adds new co-ordinate where line intersected inside the land.

        }
        // getters

        std::vector<co_ordinate> get_shape_co_oridinate(){
        return plotting;
       }

       private:
        std::vector<co_ordinate> plotting;
    };

    int main(){
      Land stage_1(vec);

       Land stage_2(stage_1);
      // plotting is replaced by 
      // new co-ordinates to store stage 2 co-ordinates
      stage_2.do_plotting_of_boundary();
      // use assignment operator
      // 
      Land stage_3(stag_2);
      // plotting is replaced by 
      // new co-ordinates to store stage 3 co-ordinates
      stage_3.do_internal_plotting();


      // get co ordinate here

      // Now to get different co-ordinate of stage 1
      // 
       std::vector<co_ordinate> hold_stage_1_co_ordinate = stage_1.get_shape_co_oridinate();

       // Now to get different co-ordinate of stage 2
      // 
       std::vector<co_ordinate> hold_stage_2_co_ordinate = stage_2.get_shape_co_oridinate();
        // Now to get different co-ordinate of stage 3
      // 
       std::vector<co_ordinate> hold_stage_3_co_ordinate = stage_3.get_shape_co_oridinate();
      }

enter image description here

I am working on project where we get land and we plot them. We represent land in x, y co-ordinate system.

stage 1 (Coarsed representation) :

For sake of simplicity lets say land we get have four boundaries.

curve boundaries : The boundaries can be curved so to represent curved boundaries we require more co-ordinate.

straight boundaries : To represent straight boundaries only two co_ordinate is sufficient.

stage 2 (Fine representation) :

Once we get the land we then divide the boundary (line segment) at certain interval. Let say at lenght of 5 meter. This will process increse the number of co_ordinate the land used to reprsent.

stage 3 (Fine and Internal co-ordinate representation) :

In this representation we draw lines between co-ordinate on the boundary from Fine representation and add new co-ordinate that intersected inside the land.

I need to save all the co_ordinate that reprsents the land.

My question is :

Do you think the way I am storing the co_ordinate in Design 1 is good design? I am doubting my self because the API design seems too verbose and probably inconsistant.

That is why I made Desing 2 whose API is simple

std::vector<co_ordinate> get_shape_co_ordinate();

But in this design I have to mange multiple object. What do you think is the correct design ? Is there better design then this?

3
  • 2
    Your code is nonsensical, so it's hard to understand the details of what you're trying to say. Since in design 2 the do_plotting_of_boundary() and do_internal_plotting() methods return void, why are you assigning them to variables? What are you trying to accomplish? – user1118321 Nov 13 '17 at 3:45
  • 1
    @user1118321 thank you for the comment and sorry about posting incomplete code. I have updated my code now – Aadarsh Sharma Nov 13 '17 at 4:23
  • The code in design 1 is not valid C++. Also, there's no need to add private: when you didn't use public: before, as the default visibility for class members is private. – Mael Nov 13 '17 at 12:28
3

What about having the co_ordinate struct hold another piece of information - which stage it as added in? So it would look something like this:

typedef enum Stage {
    Stage_Original = 0,
    Stage_Boundary,
    Stage_Internal
} Stage;

struct co_ordinate {
    double x;
    double y;
    Stage stage;
};

Then you store all the points as you normally would for stage 3. When you want to get a set of points out, you specify which points you want, and a new vector is constructed on the fly from the existing ones?

This leads to a trade-off - one which only you can decide is reasonable. If performance is an issue and memory isn't, you can store 3 vectors - one that contains all the points from stage 3, one that references the points from stage 3 that were created for stage 2, and one that references the points from stage 3 that were the original points, just like design 1. But if memory is an issue, then I recommend the solution above, which I guess is similar to design 2.

Note that in the above I mention references. The co_ordinate struct as you've defined it is 16 bytes. As I've defined it, it's probably 18-20 bytes. If your references are merely indexes, they could be unsigned shorts or unsigned ints indexing into the stage 3 vector. Therefore they would only be 2-4 bytes each. (Assuming you don't have more than 4 billion co_ordinates in the stage 3 vector.) So this is like your design 1, but a little more efficient memory-wise.

5
  • Then isn't your design similar to Design 1 (see how I have three different vector to store them). – Aadarsh Sharma Nov 13 '17 at 4:44
  • Sort of, depending on your goals. Maybe that's how I should have phrased it? Basically, if you're more concerned about performance, then Design 1 seems completely reasonable to me. But if you need to keep memory lower, then keeping a single array with all the points and marking which stage they're from takes less memory. I'll update my answer to explain that better. – user1118321 Nov 13 '17 at 4:47
  • thank you for pointing out the memory issue in detail. One last question is, in Design 1 the API seems too verbose to me (three different function to get three different stage of co-ordinate) , I guess there is no way around it then right? – Aadarsh Sharma Nov 13 '17 at 5:04
  • You could have a single function like getPoints(Stage stage) and it would return the appropriate array based on the stage variable. – user1118321 Nov 13 '17 at 5:06
  • nicee!!! thank you again for bearing with my questions :) – Aadarsh Sharma Nov 13 '17 at 5:13
1

If more typesafety is not problematic to you then I suggest solution along this one:

#include <iostream>
#include <vector>
#include <tuple>

struct Coords
{
    double x;
    double y;
};

enum class LandStages
{
    // some descriptive names, i will use StageN for simplicity
    Stage1,
    Stage2,
    Stage3
};

template <LandStages StageN>
class Land;

class LandBase
{
public:
    LandBase() = default;
    LandBase(std::vector<Coords> coords) :
        m_coords(std::move(coords))
    {
    }

    const std::vector<Coords>& coords() const
    {
        return m_coords;
    }
protected:
    std::vector<Coords> m_coords;
};

template <>
class Land<LandStages::Stage3> : public LandBase
{
public:
    using LandBase::LandBase;
};

template <>
class Land<LandStages::Stage2> : public LandBase
{
public:
    using LandBase::LandBase;

    Land<LandStages::Stage3> nextStage() const 
    {
        // simple doubleing of the vector, just to show that something is being done
        std::vector<Coords> next;
        next.reserve(m_coords.size()*2);
        for(const auto& c : m_coords)
        {
            next.emplace_back(c);
            next.emplace_back(c);
        }
        return {std::move(next)};
    }
};

template <>
class Land<LandStages::Stage1> : public LandBase
{
public:
    using LandBase::LandBase;

    Land<LandStages::Stage2> nextStage() const 
    {
        std::vector<Coords> next;
        next.reserve(m_coords.size()*2);
        for(const auto& c : m_coords)
        {
            next.emplace_back(c);
            next.emplace_back(c);
        }
        return {std::move(next)};
    }
};

template <class LandT>
void printLand(const LandT& land)
{
    for(const auto& p : land.coords())
    {
        std::cout << '(' << p.x << ", " << p.y << ") ";
    }
    std::cout << '\n';
}

template <LandStages... Stages>
class LandProcess
{
    template <size_t First, size_t Second, size_t... Tail>
    void init(std::index_sequence<First, Second, Tail...>)
    {
        std::get<Second>(m_stages) = std::get<First>(m_stages).nextStage();
        init(std::integer_sequence<size_t, Second, Tail...>{});
    }

    template <size_t... Tail>
    void init(std::index_sequence<Tail...>)
    {
    }
public:

    LandProcess(std::vector<Coords> coords)
    {
        std::get<0>(m_stages) = decltype(std::get<0>(m_stages))(coords);
        init(std::make_index_sequence<sizeof...(Stages)>{});
    }

    template <LandStages N>
    const Land<N>& getStage() const
    {
        return std::get<Land<N>>(m_stages);
    }

private:

    std::tuple<Land<Stages>...> m_stages;
};
using FullLandProcess = LandProcess<LandStages::Stage1, LandStages::Stage2, LandStages::Stage3>;

int main()
{
    FullLandProcess process({{0, 0}, {1, 0}, {1, 1}, {0, 1}});
    printLand(process.getStage<LandStages::Stage1>());
    printLand(process.getStage<LandStages::Stage2>());
    printLand(process.getStage<LandStages::Stage3>());
}

demo: http://coliru.stacked-crooked.com/a/01045563191eec28

Basically it works by having each step as a separate type (so you don't have to worry about doing the same routine twice for example, provided you make sure the input is valid). Most of the code is template fun to make a reasonable class for representing the whole process (which in this case is fairly easily customizable, you can include only some stages)

This solution, while being safer, may be more problematic for you if you want to dynamically request specific stages (would require a conditional statement and possibly runtime polymorphism) or your stage pipeline is more complicated.

Also, contrary to method presented by user1118321, this requires copying identical points (but can be made not to, though it would complicate the code).

1

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.