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

Classes and Types for EventStream Puzzler #128

Open
wants to merge 34 commits into
base: kameron-eventstream-puzzler
Choose a base branch
from

Conversation

KameronKeller
Copy link

@KameronKeller KameronKeller commented Apr 3, 2024

What this is:

  • Implement classes that are expected to be used for implementing the algorithm
  • models/highestregionlocator.ts defines a class that receives an event stream and will have a method that can locate the highest region in the dataset
    • The locateHighestRegion() function exists as a placeholder and will throw an error if called
  • models/region.ts defines a class that can add up the scores of the app events that are added to it
  • index.ts now has a main function and a console.log as a placeholder to verify it is being called when the program is being run
  • types.ts defines types for AppEvents, EventStream data, and the dictionary for the Scores class
  • index.test.ts defines high-level tests for validating that the classes created above successfully interface with each other

Issue:

N/A

Media:

EventStreamPuzzler.mov

Steps to Verify:

  • Clone the branch
  • Run yarn install
  • Run yarn test

import { AppEvent } from "./appevent";

export class EventStream {
events: AppEvent[];
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am curious if this is an appropriate way to assign a type to a variable. AppEvent is a class - would it be better to define an interface or type that defines the type that events should be?

src/assets/sampleData.ts Outdated Show resolved Hide resolved
src/index.test.ts Outdated Show resolved Hide resolved
src/index.test.ts Outdated Show resolved Hide resolved
src/index.test.ts Outdated Show resolved Hide resolved
src/index.test.ts Outdated Show resolved Hide resolved
src/index.ts Outdated Show resolved Hide resolved
src/models/appevent.ts Outdated Show resolved Hide resolved
src/models/scores.ts Outdated Show resolved Hide resolved
src/types.ts Outdated
@@ -0,0 +1,10 @@
export interface AppEventInterface {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is the difference between interface and type?

why would you use one versus another, in typescript? based on this, does it make sense to define these as types, or interfaces?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Switched to types, primarily because interfaces can be "merged" together which could be unexpected

import { Region } from './region'
import { EventStream } from '../types'

export class HighestRegionLocator {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

camelCase the file

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed filename to camel case

@@ -1,7 +1,91 @@
//write tests here
// import { AppEvent } from './models/appevent'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

avoid dead code

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dead code removed

const eventStream: EventStream = sampleData

it ('instantiates a HighestRegionLocator object', () => {
const highestRegionLocator = new HighestRegionLocator(eventStream)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no need to test instance

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed unneeded instance test


export class Region {
regionEvents: AppEvent[] = []
score: number = 0
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we need this?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed score member variable as it was not being used and won't be needed

@KameronKeller KameronKeller deleted the branch kameron-eventstream-puzzler April 5, 2024 21:29
@KameronKeller KameronKeller reopened this Apr 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants