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

Add analyzer for resistor-color #13

Closed
SleeplessByte opened this issue May 27, 2019 · 6 comments
Closed

Add analyzer for resistor-color #13

SleeplessByte opened this issue May 27, 2019 · 6 comments

Comments

@SleeplessByte
Copy link
Member

SleeplessByte commented May 27, 2019

This issue is for discussion and assignment for the resistor-color core exercise in the javascript track.

🔗 implementation | mentor-notes | problem-specification


This exercise introduces the student to Arrays as well as showing that you
can export multiple bindings at once. Solutions using an Object will actively be discouraged as speed is not a good reasoning here.

Optimal solution(s)

export const COLORS = [
  'black', 'brown', 'red', 'orange', 'yellow', 'green',
  'blue', 'violet', 'grey', 'white',
];

export const colorCode = color => COLORS.indexOf(color)

Approvable alternatives are:

  • using an arrow function expression

SHOULD comment and disapprove

Cases we SHOULD comment on with this analyzer:

  • Manually doing indexOf
  • A set of conditionals or a switch
  • Using the obscure bind to bind Array.prototype.indexOf

COULD comment (tips)

Cases we COULD comment on with this analyzer:

  • Doing an export with a specifier (at the bottom of the file) can be done export const inline
@zeckdude
Copy link
Contributor

@SleeplessByte I'd like to tackle this one.

@zeckdude
Copy link
Contributor

zeckdude commented May 31, 2019

I'm digging through the code and found the fixtures for each exercise. Got a few questions about those:

  1. Where are the test fixtures coming from? I highly doubt those were manually added. There's 499 different approaches to solving the two-fer exercise in there. That's wild.
  2. I can see the fixtures being used in the batch-runner to display general stats, but I think they would be useful for unit tests where we can assign an expected result type for each of the fixtures and if someone changes the code at a later time, the tests should catch it automatically if one of the tests no longer passes. Thoughts?
  3. I got the batch-runner to work by running node bin/batch-runner two-fer. It outputs a table with general data about each possible result type. I would really like to use the batch-runner to get a list of each fixture in each category. So then I see exactly in which category a specific fixture is being placed. Perhaps I'm missing something, but it appears that currently the only way to verify that certain fixtures are showing for a certain result type, you need to run it manually on a specific fixture. I'm hoping there's a better way, more automated way, than that. 🙏

@SleeplessByte
Copy link
Member Author

SleeplessByte commented May 31, 2019

@zeckdude sure! Before you start writing code, please lay down the approach you'll take here. That gives us some time to merge #17 and #18.

Where are the test fixtures coming from? I highly doubt those were manually added. There's 499 different approaches to solving the two-fer exercise in there. That's wild.

These are the first 500 submission provided by Jeremy to me. I will ask Jeremy for resistor-color fixtures. If you run `node bin/batch-statistics you'll find the following metrics. Valid here means it was parseble and unique is unique solutions based on AST (no whitespace and no comments). (original source message on slack)

edit: #24 done and done.

total unique valid invalid
500 358 485 15

The statistics for the runner back then was this -- should be slightly different now:

Statistics

Status Count Comments Unique Avg Median Total
Approve (optimal) 7 0 0 411ms 414ms 0s
Approve (comment) 0 0 0 0ms 0ms 0s
Disapprove (comment) 355 16 7 413ms 414ms 14s
Refer to mentor 121 2 2 413ms 415ms 5s
Total 483 17 8 413ms 415ms 19s

I can see the fixtures being used in the batch-runner to display general stats, but I think they would be useful for unit tests where we can assign an expected result type for each of the fixtures and if someone changes the code at a later time, the tests should catch it automatically if one of the tests no longer passes. Thoughts?

#9 and specifically #9 (comment)

I got the batch-runner to work by running node bin/batch-runner two-fer. It outputs a table with general data about each possible result type. I would really like to use the batch-runner to get a list of each fixture in each category. So then I see exactly in which category a specific fixture is being placed. Perhaps I'm missing something, but it appears that currently the only way to verify that certain fixtures are showing for a certain result type, you need to run it manually on a specific fixture. I'm hoping there's a better way, more automated way, than that. 🙏

I don't quite understand what you're asking for. The batch runner outputs the analysis.json files into the fixture directories which you can use. Would you like a table of "fixture number" -> "output status" -> "output comments"?

SleeplessByte added a commit that referenced this issue Jun 6, 2019
SleeplessByte added a commit that referenced this issue Jun 6, 2019
SleeplessByte added a commit that referenced this issue Jun 6, 2019
@SleeplessByte
Copy link
Member Author

@zeckdude let me know if you have further comments. If not, I might go ahead and add a minimal solution for resistor-color analyzer which you can use as a basis.

@SleeplessByte
Copy link
Member Author

#26

Raw output

{
  "invalid": 0,
  "valid": 498,
  "total": 498,
  "unique": 309
}

Parsing statistics

This is the number of unique Abstract Syntax Trees after stripping commentary,
location data (whitespace) and other tokens.

total unique valid invalid
498 309 498 0

@SleeplessByte
Copy link
Member Author

Since we have an initial analyzer, I'm closing this issue.

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

No branches or pull requests

2 participants