Arquisoft / viade_en3a

Viade En3A
https://arquisoft.github.io/viade_en3a/
1 stars 4 forks source link

MyRoute creation can be improved #162

Closed oscar134 closed 4 years ago

oscar134 commented 4 years ago

When adding the total route distance I found myself changing code in two different places. One is the MyRoute constructor where the new properties are added. But when you add new fields here, you have also to add them in the same way in the "modifyFromJsonLd()" method. This is the method where the routes are recreated once retrieved from the pod. I think this can be improved, maybe calling to the constructor and parsing the file on an external method. I believe this is important because MyRoute class will be changed frequently according to ViadeSpec issues, so it should be easy to change. What do you think about this? Any ideas to improve it?

fincamd commented 4 years ago

Well you are totally right @oscar134. That is something I had to deal with when improving the model so that it could handle both pathways to a route creation.

First --> We need to be able to create a route from the set of points that the mapcreation thing gives us. Second --> We need to use the same constructor to create routes parsed from external files.

The problem is that JavaScript doesn't feature multiple constructor for classes... So I am thinking of a solution which would be to serialize the points that the map generates to a json object and then pass it as a parameter to the constructor so it does what you mention. This way we can give a uniform treatment to the route creation process. It wouldn't matter whether we receive them from the outside or these are generated in our application.

What do you think?

oscar134 commented 4 years ago

You don't have multiple constructors, but you can use default parameters to simulate it if it's necessary. Many solutions can be valid in this case.

fincamd commented 4 years ago

What would you propose then. Maybe we can use those default parameters that you talk about.

Thinking about it, I realise that it would need to check the value we receive right? Maybe I didn't understand your proposal.

oscar134 commented 4 years ago

I mean, with default parameters you can simulate more than one constructor. For example: constructor ( a , b = 3) is like having constructor(a){ this.a = a; this.b=3 } and constructor (a,b) {...}

fincamd commented 4 years ago

Ohh. I see now, thanks. I will use that when revisiting the route creation.

fincamd commented 4 years ago

With some thinking behind the scenes, many tests and a bit of common sense over the analysis of the tradeoff this suppose, we have decided not to try and solve this issue. WontFix