HW19-OOP1

[kolarus]

• This does not look good

• all Slider methods are actually just written in constructor function, they should be on prototype

• it is better to assign such expressions to variables with meaningful name

• you should never use expressions like this, just wrap it with extra if

• it is better to assign function result to a variable and then pass it, instead of writing function call as an argument

• you can just make toLeft an arrow function to avoid all that magic with wrappers and .call

• you could have assign more methods to Slider and then inherit them in other sliders instead of copying

• it is better to make css animations, but that would be fine as well if you can use less magic values and assign name to keyframes to make it easier to read

• sliders should be more generic, a lot of values you've used are hardcoded to work with the specific slider on specific page, the whole point of OOP to make as much abstraction as it is possible and avoid specificity

[vasyabester]

Hi Bohdan! Thanks for checking. About 'all Slider methods are actually just written in constructor function, they should be on prototype'. Task was about functional inheritance. In example with functional inharitance there wasn't adding methods to prototype. So I also didn't do it. As I understood it belongs to inheritance by prototypes. About 'it is better to assign function result to a variable and then pass it, instead of writing function call as an argument'. Could you please explain a little bit more? About 'you could have assign more methods to Slider and then inherit them in other sliders instead of copying'. The task was to add only two methods in slider: 'infinity swiping' and 'managing with buttons'. With other comments I'm argee)) I could did it better. Thanks.

пн, 30 дек. 2019 г. в 13:09, Bohdan notifications@github.com:

-

This does not look good [image: image] https://user-images.githubusercontent.com/21310184/71579006-2d1d1b80-2b03-11ea-8bf9-bb41ee10da4c.png

all Slider methods are actually just written in constructor function, they should be on prototype

it is better to assign such expressions to variables with meaningful name [image: image] https://user-images.githubusercontent.com/21310184/71579088-9dc43800-2b03-11ea-91c1-e0e1dff353e3.png [image: image] https://user-images.githubusercontent.com/21310184/71579352-dd3f5400-2b04-11ea-8132-975a000f458a.png

you should never use expressions like this, just wrap it with extra if [image: image] https://user-images.githubusercontent.com/21310184/71579180-1a571680-2b04-11ea-94dd-c43a5fd5c639.png

it is better to assign function result to a variable and then pass it, instead of writing function call as an argument [image: image] https://user-images.githubusercontent.com/21310184/71579216-3b1f6c00-2b04-11ea-8ab9-da505f253c22.png

you can just make toLeft an arrow function to avoid all that magic with wrappers and .call [image: image] https://user-images.githubusercontent.com/21310184/71579270-8174cb00-2b04-11ea-97f6-a9eee299e0eb.png

you could have assign more methods to Slider and then inherit them in other sliders instead of copying

it is better to make css animations, but that would be fine as well if you can use less magic values and assign name to keyframes to make it easier to read [image: image] https://user-images.githubusercontent.com/21310184/71579411-14ae0080-2b05-11ea-88dc-65c512f22ca8.png

sliders should be more generic, a lot of values you've used are hardcoded to work with the specific slider on specific page, the whole point of OOP to make as much abstraction as it is possible and avoid specificity

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/vasyabester/epam_fe_2019_tyurin/issues/17?email_source=notifications&email_token=ANOUJ3MEXI6RT55H3DM3CQ3Q3HJF3A5CNFSM4KBLPA42YY3PNVWWK3TUL52HS4DFUVEXG43VMWVGG33NNVSW45C7NFSM4IDKLT4A, or unsubscribe https://github.com/notifications/unsubscribe-auth/ANOUJ3NE3C3CXMFPIJU57XDQ3HJF3ANCNFSM4KBLPA4Q .

[kolarus]

Hi, about functional inharitance and slider methods, you are 100% right, that was my mistake and it is even better that you are have not just agreed with my comments, but tried to get it right. Thank you! About 'it is better to assign function result to a variable and then pass it, instead of writing function call as an argument': function(test, document.querySelector('.class')) could be consy domNode = document.querySelector('.class') function(test, domNode) this way when i am reading the line of code with function call i can just read the name of an argument and if it is descriptive enough i will not need to go "deeper" into the logic of where it is comming from

[vasyabester]

Hello Bohdan! Thanks for answer! And Happy coming New Year!;)

вт, 31 дек. 2019 г. в 8:11, Bohdan notifications@github.com:

Hi, about functional inharitance and slider methods, you are 100% right, that was my mistake and it is even better that you are have not just agreed with my comments, but tried to get it right. Thank you! About 'it is better to assign function result to a variable and then pass it, instead of writing function call as an argument': function(test, document.querySelector('.class')) could be consy domNode = document.querySelector('.class') function(test, domNode) this way when i am reading the line of code with function call i can just read the name of an argument and if it is descriptive enough i will not need to go "deeper" into the logic of where it is comming from

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/vasyabester/epam_fe_2019_tyurin/issues/17?email_source=notifications&email_token=ANOUJ3NYK4W2I4ARV3W3NUDQ3LPAPA5CNFSM4KBLPA42YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEH3YX7Y#issuecomment-569871359, or unsubscribe https://github.com/notifications/unsubscribe-auth/ANOUJ3MZABSDTYFDFWGZDL3Q3LPAPANCNFSM4KBLPA4Q .