Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement core module basics [#9] - helpers implementation #14

Merged
merged 2 commits into from Oct 24, 2019

Conversation

@rebzden
Copy link
Collaborator

rebzden commented Oct 24, 2019

No description provided.

packages/core/src/core.ts Outdated Show resolved Hide resolved
@@ -1 +1,49 @@
export const x = true
export const isEmpty = value => {
if (isNothing(value) || value === "") {

This comment has been minimized.

Copy link
@ulfryk

ulfryk Oct 24, 2019

Member

let's use ' always

This comment has been minimized.

Copy link
@rebzden

rebzden Oct 24, 2019

Author Collaborator

fixed


export const isNothing = value => {
return value == null;
};

This comment has been minimized.

Copy link
@ulfryk

ulfryk Oct 24, 2019

Member

=> { return x } --> => x

This comment has been minimized.

Copy link
@rebzden

rebzden Oct 24, 2019

Author Collaborator

fixed


export const noop = () => {};

export const isNothing = value => {

This comment has been minimized.

Copy link
@ulfryk

ulfryk Oct 24, 2019

Member

arg type

This comment has been minimized.

Copy link
@rebzden

rebzden Oct 24, 2019

Author Collaborator

fixed

return false;
}

if (isFunction(a.equals) && isFunction(b.equals)) {

This comment has been minimized.

Copy link
@ulfryk

ulfryk Oct 24, 2019

Member

i'd suggest adding interface defining Setoid and function isSetoid(a: ?) a is Setoid to use here ;)

This comment has been minimized.

Copy link
@rebzden

rebzden Oct 24, 2019

Author Collaborator

will do in the future

};

export const compose = (f: Function, g: Function) => {
return function(x: unknown) {

This comment has been minimized.

Copy link
@ulfryk

ulfryk Oct 24, 2019

Member

(f: ?, g: ?) => (x: ?): ? => f(g(x))

  • remove obsolete function and return statements
  • add proper generic types here ;)

This comment has been minimized.

Copy link
@rebzden

rebzden Oct 24, 2019

Author Collaborator

split the file, generic will be added with more infrastructure added later

return function(x: unknown) {
return f(g(x));
};
};

This comment has been minimized.

Copy link
@ulfryk

ulfryk Oct 24, 2019

Member
  • split this file please - each helper in it's own file
  • Setoid and it's related functions grouped into one file

This comment has been minimized.

Copy link
@rebzden

rebzden Oct 24, 2019

Author Collaborator

as above

@ulfryk ulfryk added this to the 1.0.0 (#hacktoberfest) milestone Oct 24, 2019
@ulfryk

This comment has been minimized.

Copy link
Member

ulfryk commented Oct 24, 2019

Maybe add issue number to commit/PR title (it helps to tie things together later)

@rebzden rebzden force-pushed the core-helpers branch 3 times, most recently from d1f2c39 to 7851ed4 Oct 24, 2019
@rebzden rebzden force-pushed the core-helpers branch from 7851ed4 to 2bec5ff Oct 24, 2019
@rebzden rebzden changed the title Core helpers Implement core module basics #9 - helpers implementation Oct 24, 2019
@rebzden rebzden changed the title Implement core module basics #9 - helpers implementation Implement core module basics [#9] - helpers implementation Oct 24, 2019

export const equals = (a: unknown) => (b: unknown) => areEqual(a, b)

const areEqual = (a: any, b: any) =>

This comment has been minimized.

Copy link
@ulfryk

ulfryk Oct 24, 2019

Member

this function is obcolete

This comment has been minimized.

Copy link
@rebzden

rebzden Oct 24, 2019

Author Collaborator

fixed

/* tslint:disable:no-empty */
export const noop = () => {}

export const compose = (f: Function, g: Function) =>

This comment has been minimized.

Copy link
@ulfryk

ulfryk Oct 24, 2019

Member

duplicated

This comment has been minimized.

Copy link
@rebzden

rebzden Oct 24, 2019

Author Collaborator

fixed

export const isNothing = (value: any) => value == null

/* tslint:disable:no-empty */
export const noop = () => {}

This comment has been minimized.

Copy link
@ulfryk

ulfryk Oct 24, 2019

Member

duplication

This comment has been minimized.

Copy link
@rebzden

rebzden Oct 24, 2019

Author Collaborator

fixed

export const isFunction = (f: any) =>
Boolean(f && f.constructor && f.call && f.apply);

export interface Setoid<A> {

This comment has been minimized.

Copy link
@ulfryk

ulfryk Oct 24, 2019

Member

move it out from here to own space

This comment has been minimized.

Copy link
@rebzden

rebzden Oct 24, 2019

Author Collaborator

fixed

a === b || (isNaN(a) && isNaN(b))

const areFunctionsEqual = (a: any, b: any) =>
isFunction(a.equals) && isFunction(b.equals) && a.equals(b)

This comment has been minimized.

Copy link
@ulfryk

ulfryk Oct 24, 2019

Member

use isSetoid(a) instead

This comment has been minimized.

Copy link
@rebzden

rebzden Oct 24, 2019

Author Collaborator

fixed

/* tslint:disable:no-empty */
export const noop = () => {};

export const compose = (f: Function, g: Function) => (x: unknown) => f(g(x));

This comment has been minimized.

Copy link
@ulfryk

ulfryk Oct 24, 2019

Member

this one should be strongly typed

This comment has been minimized.

Copy link
@rebzden

rebzden Oct 24, 2019

Author Collaborator

fixed

@rebzden rebzden force-pushed the core-helpers branch 2 times, most recently from 8f27b90 to 1e9a2db Oct 24, 2019
export const isEmptyObject = (value: any) =>
typeof value === 'object' && Object.keys(value).length === 0

export const isNothing = (value: any) => value == null

This comment has been minimized.

Copy link
@ulfryk

ulfryk Oct 24, 2019

Member

I'd suggest extracting isNothing to separate file

const areSimplyEqual = (a: any, b: any) =>
a === b || (isNaN(a) && isNaN(b))

const areFunctionsEqual = (a: any, b: any) =>

This comment has been minimized.

Copy link
@ulfryk

ulfryk Oct 24, 2019

Member

Maybe areSetoidsEqual

@rebzden rebzden force-pushed the core-helpers branch 3 times, most recently from 6e44f81 to 0f65497 Oct 24, 2019
@@ -0,0 +1,6 @@
import { areSetoidsEqual } from './setoid'

This comment has been minimized.

Copy link
@ulfryk

ulfryk Oct 24, 2019

Member

name file equals and add unit tests

@rebzden rebzden force-pushed the core-helpers branch from 0f65497 to 83c6904 Oct 24, 2019
NaN,
].forEach(empty => expect(isEmpty(empty)).to.be.false)
})
given`Non empty expect value to be true`(() => {

This comment has been minimized.

Copy link
@ulfryk

ulfryk Oct 24, 2019

Member

given -> when -> then
given -> expect

I'd suggest adding expect helper to gherkin helpers

so it looks like:

let emptyValues: any[];

given `some empty values` (() => {
  emptyValues = […]
})

expect `them to pass "isEmpty" test` (() => {
  emptyValues.forEach(v => {
     xpct(isEmpty(v)).to.be.true
  })
})

or

let emptyValues: any[];
let testResults: boolean[];

given `some empty values` (() => {
  emptyValues = […]
})

when `they are tested with "isEmpty"` (() => {
  testResults = emptyValues.map(isSome) // or use filter and assert count
})

then `they all pass the test` (() => {
  expect(testResults).to.deep.equal([true, true, …])
})
expect(equals({})({})).to.equal(false)
expect(equals(3)([3])).to.equal(false)
expect(equals(3)('3')).to.equal(false)
})

This comment has been minimized.

Copy link
@ulfryk

ulfryk Oct 24, 2019

Member

Follow similar rules to ones in isEmpty

expect(isNothing(false)).to.be.false
expect(isNothing(true)).to.be.false
expect(isNothing([])).to.be.false
expect(isNothing({})).to.be.false

This comment has been minimized.

Copy link
@ulfryk
Jakub Strojewski
@ulfryk ulfryk dismissed their stale review Oct 24, 2019

fixed

@ulfryk
ulfryk approved these changes Oct 24, 2019
@ulfryk ulfryk self-assigned this Oct 24, 2019
@ulfryk ulfryk merged commit 8ea2470 into develop Oct 24, 2019
1 check passed
1 check passed
Travis CI - Pull Request Build Passed
Details
@ulfryk ulfryk deleted the core-helpers branch Oct 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.