My solutions #11

danvk · 2020-05-07T22:41:10Z

First off, amazing work! I'd been thinking of creating homework / exercises / problem sets for Effective TypeScript, but given the work you've put into this I think I'll just be linking to yours instead.

Feedback on specific issues below, but some high level comments:

As others have pointed out, in many cases the tests are looser than they should be and allow incorrect or imprecise types through. It's just as important to reject incorrect types as it is to accept correct ones. A tool like tsd could help with assertions here, but the new @ts-expect-error assertion (introduced in TS 3.9) might be your best bet. It produces an error unless the next line has a type error. So:

function square(x: any) {
  return x * x;
}

// @ts-expect-error
square('ten');

will fail unless you pick a parameter type precise enough to exclude ten.

Especially in the earlier exercises, I sometimes had a hard time figuring out what you were looking for in a solution until I saw which article you linked to as a hint. There are often many ways to do things in TypeScript (user-defined type guards, in tests, tagged unions) and some more guidance about which you were trying to teach might have been helpful.
Unless I missed it, I don't think tagged unions are ever explicitly introduced. Exercise 3 has a type field but keeps the user-defined type guards (isAdmin, isUser) which are completely unnecessary when you have a tag. Given how pervasive tagged unions are as a construct in TS, it would be useful to have an exercise that forces the user to narrow a type based on its tag:

    if (person.type === 'admin') {
        additionalInformation = person.role;
    }
    if (person.type === 'user') {
        additionalInformation = person.occupation;
    }

instead of:

    if (isAdmin(person)) {
        additionalInformation = person.role;
    }
    if (isUser(person)) {
        additionalInformation = person.occupation;
    }

It might be useful to have some more exercises around type inference, i.e. when is TypeScript able to infer types through map / filter / etc and when isn't it? Given how pervasive lodash is, it might also be useful to have some examples using that.

I'll leave more specific comments on each exercise.

danvk

Feedback on exercises 1–9, I'll leave more later!

danvk · 2020-05-07T22:42:28Z

.vscode/settings.json

@@ -0,0 +1,5 @@
+{
+  "files.exclude": {
+    "**/node_modules": false,


In exercise 10, it took me a bit to figure out that node_modules/str-utils/index.js referred to exercises/exercise-10/node_modules, not the top-level node_modules. Adding this to my vscode settings made it easier to find.

Would you be able to make a pull-request with this change?

danvk · 2020-05-07T22:43:20Z

exercises/exercise-02/index.ts

@@ -64,8 +64,8 @@ const persons: Person[] = [
 ];

 function logPerson(person: Person) {
-    let additionalInformation: string;


It's interesting to note that this annotation is not needed. See this post for details.

danvk · 2020-05-07T22:44:03Z

exercises/exercise-04/index.ts

@@ -95,9 +95,10 @@ function logPerson(person: Person) {
    console.log(` - ${chalk.green(person.name)}, ${person.age}, ${additionalInformation}`);
 }

-function filterUsers(persons: Person[], criteria: User): User[] {
+type Criteria = Partial<Omit<User, 'type'>>;


I found the hint to use mapped types a bit confusing given that you don't need them.

Partial and Omit are implemented using mapping. You can either use them directly or write a mapped type yourselves.

danvk · 2020-05-07T22:47:57Z

exercises/exercise-05/index.ts

@@ -64,11 +64,18 @@ function logPerson(person: Person) {
    );
 }

-function filterPersons(persons: Person[], personType: string, criteria: unknown): unknown[] {
+function getObjectKeys<T extends object>(o: T): (keyof T)[] {


The instructions for this are:

Implement a function getObjectKeys() which returns proper type
for any argument given, so that you don't need to cast it.

"More convenient", sure, but "proper" is too strong here, string[] really is the correct type for the reasons outlined in this StackOverflow answer and this TS issue.

There's also a discussion of this in Effective TypeScript (Item 54). I'm happy to get you a free copy if you'd like.

Happy to receive a copy if you can issue it to me.

Feel free to make a pull-request with this re-phrasing.

danvk · 2020-05-07T22:49:09Z

exercises/exercise-05/index.ts

@@ -78,6 +85,11 @@ function filterPersons(persons: Person[], personType: string, criteria: unknown)
 let usersOfAge23: User[] = filterPersons(persons, 'user', { age: 23 });
 let adminsOfAge23: Admin[] = filterPersons(persons, 'admin', { age: 23 });

+// @ts-ignore


This would be a great use of @ts-expect-error: filtering to admins shouldn't allow you to select by occupation.

danvk · 2020-05-07T22:54:26Z

exercises/exercise-05/index.ts

+// @ts-ignore
+let astronautAdmins: Admin[] = filterPersons(persons, 'admin', { occupation: 'Astronaut' });
+
+let peopleOfAge23: Person[] = filterPersons(persons, Math.random() < 0.5 ? 'user' : 'admin', {age: 23});


This call results in an error with the overloaded definitions, even though it is guaranteed to be valid. This is one of the problems with overloads, and it's a reason to prefer using conditional types. Rewriting this function to use conditional types instead of an overload is much harder but might be a good exercise for later.

You can learn more about why conditional types are preferred in this fantastic artsy post or Item 50 (Prefer Conditional Types to Overloaded Declarations) in Effective TypeScript.

danvk · 2020-05-07T22:56:00Z

exercises/exercise-07/index.ts

@@ -41,7 +41,7 @@ interface Admin {
    role: string;
 }

-type PowerUser = unknown;
+type PowerUser = Omit<User, 'type'> & Omit<Admin, 'type'> & {type: 'powerUser'};


It wasn't clear to me what you were supposed to do in this exercise if you didn't do the "higher difficulty" version.

In the normal difficulty version you manually write PowerUser type without using neither User and Admin types.

👍 The solution in the 'solutions' branch didn't work for me with TypeScript v3.9.7; I got multiple errors. The solution in this PR did work for me.

danvk · 2020-05-07T22:56:45Z

exercises/exercise-09/index.ts

@@ -71,8 +71,16 @@ type ApiResponse<T> = (
    }
 );

-function promisify(arg: unknown): unknown {
-    return null;
+function promisify<T>(fn: (callback: (arg: ApiResponse<T>) => void) => void): () => Promise<T> {


I think this is a great problem (implement promisify). The instructions could be more clear that you're expected to wrap errors in an Error object.

Also unclear to me what work there is to do here if you don't do the "higher difficulty" challenge.

Also unclear to me what work there is to do here if you don't do the "higher difficulty" challenge.

This is just a higher difficulty exercise :)

I think this is a great problem (implement promisify). The instructions could be more clear that you're expected to wrap errors in an Error object.

Feel free to provide a pull-request with this change.

danvk

OK, that's all my feedback. Thanks again for the great exercises, I had a blast doing them. I especially loved the stories about CEO and his/her friend Nick. Let me know if there's anything I can do to help with this project!

danvk · 2020-05-08T00:07:16Z

exercises/exercise-11/declarations/stats/index.d.ts

+    function GetIndexFn<T>(input: T[], comparator: (a: T, b: T) => number): number;
+    function GetElementFn<T>(input: T[], comparator: (a: T, b: T) => number): T | null;
+
+    export const getMaxIndex: typeof GetIndexFn;


It's interesting that you can use typeof Fn here to "inherit" the generic parameter. I don't think you can do such a thing with a type alias. (I tried above.)

You can use type aliases like this

type MyFunction = <T>(arg: T): T;

danvk · 2020-05-08T00:08:20Z

exercises/exercise-13/database.ts

@@ -1,13 +1,85 @@
-export class Database<T> {
+import * as fs from 'fs';


Exercise 13 was probably more work than all the others put together. I wonder if it could be split?

I'm thinking of redoing 13 and later exercises to focus more on types.

danvk · 2020-05-08T00:10:05Z

exercises/exercise-13/database.ts

+};
+
+type Unionize<T extends object> = {[k in keyof T]: {k: k, v: T[k]}}[keyof T];
+type QueryableKeys<T extends object> = Extract<Unionize<T>, {v: JsonScalar}>['k'];


This enforces that you only run queries on fields whose values are JsonScalar types. Maybe useful as an advanced extension checked with @ts-expect-error.

danvk · 2020-05-08T00:10:37Z

exercises/exercise-13/index.ts

+    // Can't use any of these operators with array types.
+    expect(
+        // @ts-ignore
+        (await usersDatabase.find({grades: {$eq: 'Magical entity'}})).map(({_id}) => _id)


Since grades is array-valued, this shouldn't be allowed. It was actually a little unclear to me from the problem statement if non-scalars were to be allowed in records, but they're valid JSON, so I didn't see why not.

Only scalars are allowed in this implementation.

danvk · 2020-05-08T00:11:00Z

exercises/exercise-13/database.ts

+        return words.every(w => r.$index[w]);
+    }
+    return Object.entries(q).every(
+        ([k, v]) => matchOp(v as FieldOp, r[k as keyof T] as any)


I did use an any here. Sorry! :)

You can make FieldOp a generic, i.e. FieldOp<T> and cast the other value to T.

danvk · 2020-05-08T00:16:47Z

exercises/exercise-13/database.ts


-    constructor(filename: string, fullTextSearchFieldNames) {
+    constructor(filename: string, fullTextSearchFieldNames: (keyof T)[]) {


It would be nice to enforce that the fullTextSearchFieldNames correspond to string-valued properties in T. One way to do this is with the Objectify / Unionize generics I defined above (I'll be writing a blog post about these soon):

type StringKeys<T extends object> = Extract<Unionize<T>, {v: string}>['k']; export class Database<T extends object> { constructor(filename: string, fullTextSearchFieldNames: StringKeys<T>[]) { // ... } }

It's pretty neat to see this working in autocomplete!

danvk · 2020-05-08T00:19:32Z

exercises/exercise-14/database.ts

    }

-    async find(query, options?): Promise<T[]> {


It wasn't clear to me why this was declared async.

To avoid synchronous file operations. You read the database when necessary.

danvk · 2020-05-08T00:21:30Z

exercises/exercise-14/database.ts

    }

-    async find(query, options?): Promise<T[]> {
-        return [];
+    async find(query: Query<T>, options?: Options<T>): Promise<Partial<T>[]> {


It would be a nice extension to have a more precise return type when options.projection is and is not specified. (It should be Promise<T[]> if it isn't and something less specific if it is.)

danvk · 2020-05-08T00:22:59Z

exercises/exercise-15/database.ts

+        return rs;
+    }
+
+    async delete(query: Query<T>) {


Given the problem description, I was a little surprised that the tests didn't check that you wrote anything to disk. This problem felt like it had more to do with Node.js and less to do with TypeScript.

danvk · 2020-05-08T00:24:03Z

exercises/exercise-15/index.ts

@@ -1,6 +1,6 @@
 import {Database} from './database';
 import * as path from 'path';
-import {promises as fs} from 'mz/fs';
+import {promises as fs} from 'fs';


I got runtime errors about fs.copyFile not being a function until I changed mz/fs → fs.

fireairforce · 2020-05-10T08:18:30Z

Awesome~

mdevils · 2020-05-11T15:57:48Z

Glad you liked it, thanks for the comments.

VSCode settings for exercise #11

mdevils · 2020-05-11T15:43:00Z

exercises/exercise-04/index.ts

@@ -95,9 +95,10 @@ function logPerson(person: Person) {
    console.log(` - ${chalk.green(person.name)}, ${person.age}, ${additionalInformation}`);
 }

-function filterUsers(persons: Person[], criteria: User): User[] {
+type Criteria = Partial<Omit<User, 'type'>>;


Partial and Omit are implemented using mapping. You can either use them directly or write a mapped type yourselves.

mdevils · 2020-05-11T15:44:12Z

exercises/exercise-05/index.ts

@@ -64,11 +64,18 @@ function logPerson(person: Person) {
    );
 }

-function filterPersons(persons: Person[], personType: string, criteria: unknown): unknown[] {
+function getObjectKeys<T extends object>(o: T): (keyof T)[] {


Happy to receive a copy if you can issue it to me.

mdevils · 2020-05-11T15:44:33Z

exercises/exercise-05/index.ts

@@ -64,11 +64,18 @@ function logPerson(person: Person) {
    );
 }

-function filterPersons(persons: Person[], personType: string, criteria: unknown): unknown[] {
+function getObjectKeys<T extends object>(o: T): (keyof T)[] {


Feel free to make a pull-request with this re-phrasing.

mdevils · 2020-05-11T15:47:43Z

exercises/exercise-07/index.ts

@@ -41,7 +41,7 @@ interface Admin {
    role: string;
 }

-type PowerUser = unknown;
+type PowerUser = Omit<User, 'type'> & Omit<Admin, 'type'> & {type: 'powerUser'};


In the normal difficulty version you manually write PowerUser type without using neither User and Admin types.

mdevils · 2020-05-11T15:49:05Z

exercises/exercise-09/index.ts

@@ -71,8 +71,16 @@ type ApiResponse<T> = (
    }
 );

-function promisify(arg: unknown): unknown {
-    return null;
+function promisify<T>(fn: (callback: (arg: ApiResponse<T>) => void) => void): () => Promise<T> {


Also unclear to me what work there is to do here if you don't do the "higher difficulty" challenge.

This is just a higher difficulty exercise :)

mdevils · 2020-05-11T15:51:14Z

exercises/exercise-12/index.ts


    1. DateDetails interface is missing
-       time related fields such as hours, minutes and
+       time-related fields such as hours, minutes and


Feel free to send me a pull-request with grammar fixes.

mdevils · 2020-05-11T15:52:14Z

exercises/exercise-13/database.ts

@@ -1,13 +1,85 @@
-export class Database<T> {
+import * as fs from 'fs';


I'm thinking of redoing 13 and later exercises to focus more on types.

mdevils · 2020-05-11T15:53:59Z

exercises/exercise-13/database.ts

+        return words.every(w => r.$index[w]);
+    }
+    return Object.entries(q).every(
+        ([k, v]) => matchOp(v as FieldOp, r[k as keyof T] as any)


You can make FieldOp a generic, i.e. FieldOp<T> and cast the other value to T.

mdevils · 2020-05-11T15:56:00Z

exercises/exercise-13/index.ts

+    // Can't use any of these operators with array types.
+    expect(
+        // @ts-ignore
+        (await usersDatabase.find({grades: {$eq: 'Magical entity'}})).map(({_id}) => _id)


Only scalars are allowed in this implementation.

mdevils · 2020-05-11T15:56:32Z

exercises/exercise-14/database.ts

    }

-    async find(query, options?): Promise<T[]> {


To avoid synchronous file operations. You read the database when necessary.

…1-vscode VSCode settings for exercise typescript-exercises#11

oahehc · 2020-05-25T22:55:38Z

exercises/exercise-11/declarations/stats/index.d.ts

+    export const getMaxElement: typeof GetElementFn;
+    export const getMinElement: typeof GetElementFn;
+    export const getMedianElement: typeof GetElementFn;
+    export const getAverageValue: typeof GetElementFn;


I think getAverageValue should be different from others

export function getAverageValue<T>(input: T[], getValue: (a: T) => number): number | null;

I am agree，there different comparator in getAverageVal

i think getAverageValue shouldn't be null

export function getAverageValue<T>( input: T[], getValue: (item: T) => any ): number; }

fabb · 2020-06-01T10:44:07Z

exercises/exercise-13/database.ts

+    {$or: Query<T>[]} |
+    {$text: string} |
+    (
+        {[field in QueryableKeys<T>]?: FieldOp}


The type can be improved by using distributive conditionals:
https://www.typescriptlang.org/docs/handbook/advanced-types.html#distributive-conditional-types

This can be used to disallow {} being passed as query. Ideally, it would also disallow passing multiple fields in one query, but TypeScript is not yet powerful enough to detect this.

https://github.com/fabb/typescript-exercises/blob/my_solutions/exercises/exercise-13/database.ts#L74

type Query<T> = FieldQuery<T> | AndQuery<T> | OrQuery<T> | TextQuery<T> type FieldQuery<T, K extends keyof T = keyof T> = K extends string ? { [k in K]: Comparator<T[K]> } : never

danvk added 17 commits May 3, 2020 11:58

Solve Problem 1

b791259

Solve Exercise 01

9eadac4

Solve exercise 3

21b1767

Solve exercise 4

1f5fb57

Complete exercise 5

c3bddcb

Complete exercise 6

4771c24

Complete exercise 7

1c0a402

Complete exercises 8 & 9

4907c99

Solve exercise 10

8bf2984

Complete Exercise 11

6e5d795

Complete exercise 12

a306590

Complete exercise 13; lots of anys

a857acf

Slightly better solution? A bit better safety

9bf443f

Going to move on to 14

a97b917

Complete exercise 14

3df596f

Complete exercise 15

dce1519

rm notes file

c7bc621

danvk commented May 7, 2020

View reviewed changes

danvk commented May 8, 2020

View reviewed changes

danvk marked this pull request as ready for review May 8, 2020 00:25

TheDoctor0 mentioned this pull request May 11, 2020

VSCode settings for exercise #11 #15

Merged

mdevils added a commit that referenced this pull request May 11, 2020

Merge pull request #15 from TheDoctor0/exercise-11-vscode

1990792

VSCode settings for exercise #11

mdevils reviewed May 15, 2020

View reviewed changes

filipows pushed a commit to filipows/typescript-exercises that referenced this pull request May 16, 2020

Merge pull request typescript-exercises#15 from TheDoctor0/exercise-1…

b20d3a1

…1-vscode VSCode settings for exercise typescript-exercises#11

oahehc reviewed May 25, 2020

View reviewed changes

fabb reviewed Jun 1, 2020

View reviewed changes

mdevils closed this Aug 13, 2020

		@@ -1,13 +1,85 @@
		export class Database<T> {
		import * as fs from 'fs';


		constructor(filename: string, fullTextSearchFieldNames) {
		constructor(filename: string, fullTextSearchFieldNames: (keyof T)[]) {

Uh oh!

My solutions #11

My solutions #11

Uh oh!

Conversation

danvk commented May 7, 2020

Uh oh!

danvk left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

danvk May 7, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

danvk left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fireairforce commented May 10, 2020

Uh oh!

mdevils commented May 11, 2020

danvk May 7, 2020 •

edited

Loading