mrhut10 / todaysCuriosity

a set of single_image_processing effects inspired by the art work by Kensuke Koike. See live demo on
https://todayscuriosity.netlify.com/
5 stars 9 forks source link

refactor library to be stateless/functional??? #16

Closed mrhut10 closed 6 years ago

mrhut10 commented 6 years ago

This is certainly more of an opinion than a fact, but wanted to explore the topic.

stored state within the library is it necessary? I'll start with con's for refactoring the state away ( so i'm not completely bias )

Refactor Con's

Refactor Pro's (all opinions not fact)

anyway interested what @khutten and any others thoughts on it.

ghost commented 6 years ago

Can you please tell me what you mean by "we already create new instance on each use/update currently". New instance of what?

I terms of state I agree that the divisions, offsets and reduction ratio really don't need to be stored in object state and can be given in a function, but the input image would seems silly to pass that to it each time, I mean I guess that it can be done. One thing that would be less efficient is that the library makes a separate image with brighter pixels to paint on the original image to show which pixels are being used for the output, without state this would be calculated each time, which it currently only does when it's given a new input image. I guess there might be better ways of highlighting these pixels that I haven't considered. Like I know you can draw rectangles with the canvas, if they can be drawn with an alpha channel that might be better anyway. Something I should look into.

Ultimately, If you want to make it functional lead the charge. It's funny I refactored it to use classes and now we are probably going to refactor again to make it functional. But that's all good, part of the fun.

Also I've never heard anyone say "nice and verbosely" before haha 😂

mrhut10 commented 6 years ago

Ok maybe I've miss understood a few things here.

  1. i thought app created a new instance of todaysCuriosity object/class on each update but now that I'm looking at that doesn't appear to be the case which is a big fault in my argument which might mean we want to throw my whole argument out.
  2. divisions, offsets and reduction ration being passed in functionally would prevent people messing with state so sounds good not that we need to worry about this much serves someone write for not reading our non existent documentation, lol.
  3. if we want to preventing sending the image multiple times we could curry the function, sometimes this is called partially or delayed execution although by strict definition it is different.
  4. drawing over the top with alpha, makes me wonder if we can draw svgs inside a canvas also, although that's likely way overkill.

anyway its I'm passionate about I'll have a go at refactoring but won't waste much of anyone time talking about it as we have other things to work out.

mrhut10 commented 6 years ago

if i do have a go at this I'll also implement it in typescript

ghost commented 6 years ago

Cool cool, I'm happy with what ever really.

mrhut10 commented 6 years ago

@khutten mentioned concern the other day about people could mess with state which could brake the class operation a little, just wanted to show you something you may not have heard of called currying functions.

// example

const add = x => y => x+y;
add(3)       // will return a function that will add three to any parem you send it ( partially executed )
add(3)(7)  // will return number 10 and is now fully exicuted

functional languages curry functions automatically however can be easily done as above example within each function manually, or a harder way but potentially more convenient in the future is create a higher order curry function which will also partially executed function.

one application for this with what we are doing would be the example below it is impossible to break the state as enforces application of the problem in a defined order.

todaysCuriosity = (inputDiv, outputDiv) => {
  // do any initial setup
  return (baseImage) => {
    //do any inital work to display input image
    return (effect) => {
      //load defaults on effect and display output image
      return (effectSettings){
        //output the image
        // the function is now fully exicuted
      }
    }
  }
}

Hopefully that isn't too confusing?? In practice how you would use the library i need to think a little more about it before i have any particular recommendation.

mrhut10 commented 6 years ago

just adding to this i've looked it up to confirm.

this is talking about scoping functions so they aren't accessible from outside but still applies

http://es6-features.org/#BlockScopedFunctions

hope this is helpful??

ghost commented 6 years ago

Right, it kinda makes sense 🤷‍♀️

Sounds good and very interesting. But I don't know much about functional programing. Can't we just make the whole app with a single .map?

mrhut10 commented 6 years ago

yep i guess, and you know i said previously. I like that you've got your head around the library part. I should just focus my attention on getting the front end nice seeing the library is actually working. mobile friendly #3 is certainly more of a priority. When my attention can get back on this ( after Tuesday night ) that's what I'll focus on. :) have a great Saturday @khutten

ghost commented 6 years ago

I got this git presentation next Thursday, once I'm done preping for that I'll likely give this project a little more ❤️.