assistify / diary

Apache License 2.0
0 stars 1 forks source link

Refactored URL generation/parsing to be used in the library part #62

Closed jschirrmacher closed 5 years ago

jschirrmacher commented 5 years ago

I agree, that statefulUrl might be a better name. For the functions, I'd then prefer just generate() and getDiaryInfo() because they are methods on the statefulUrl already like in these examples:

const url = statefulUrl.generate({...})
const diaryInfo = statefulUrl(url)
jschirrmacher commented 5 years ago

Btw I think URL is more specific than URI because it contains the protocol and server name (and even the protocol, if different from the default), so we really have a 'location' here.

mrsimpson commented 5 years ago

... because they are methods on the statefulUrl already

well, this is something I'm also wondering: if you make it a functional library

export function generate()...
export function getDiaryInfo()

then, for the consumer the context if not visible anymore:

import {generate} from 'statefulUrl';

generate() // what?

if you export an object, you can consume it well-readable (statefulUrl.generate()), but then it looks as if it was a class which needs to be instantiated first.

Given those options, I'd rather prefer to export pure functions (without a wrapping "class"). This at least allows the consumer to adapt the name and makes clear, there's no instantiation neccessary

import {generate as generateStatefulUrl} from 'statefulUrl'
mrsimpson commented 5 years ago

Btw I think URL is more specific than URI because it contains the protocol and server name (and even the protocol, if different from the default), so we really have a 'location' here.

😆 - I expected this comment. I was just playing it safely: URI is more generic, but please leave it URL if you prefer!

jschirrmacher commented 5 years ago

Given those options, I'd rather prefer to export pure functions (without a wrapping "class").

It already is 😀

jschirrmacher commented 5 years ago

I should have completed the requested changes now, so please re-check and approve if I didn't miss anything