`edge_insets.dart` should be reviewed

This issue has been tracked since 2022-11-23.

Here is the link to the file: https://github.com/flutter/flutter/blob/master/packages/flutter/lib/src/painting/edge_insets.dart#L23

Some things found about the classes in this file.

-In edge_insets.dart on lines 894 and 997 the assert(direction != null); is called even tho on lines 893 and 996 we accept a nullable parameter. These assertion is redundant or we shouldn't accept a nullable parameter.

-The constructor EdgeInsetsDirectional.all is more expensive than EdgeInsets to resolve and this hasn't been documented. Also I don't think there's a proper reason for EdgeInsetsDirectional.all to exist in the codebase.
I propose that EdgeInsetsDirectional.all is removed. Same goes for EdgeInsetsDirectional.symmetrical.

-The way inheritance is used here could be argued as bad practice. Every sibling class(whose parent is EdgeInsetsGeometry) has to be aware of the EdgeInsets class.
I propose that someone with deeper knowledge of the framework or the file itself would review the file with this thought in their mind.

-Someone experienced should review the whole file imo

exaby73 wrote this answer on 2022-11-24

Hello @patrikheinonen. Could you include permalinks to the code in question for easier reference? You could also include the changes you would expect or better yet, file a PR if you want :)

patrikheinonen wrote this answer on 2022-11-24

Yeah @exaby73 Im very new to this open source stuff. This is actually the first issue I have ever raised so I didn't know about permalinks. I have now updated my initial comment.

I just tried to point someones attention to the file. I will read the contributor rules etc and try to make a PR 👍
what comes to the third point in my initial comment. The refactoring should prob be discussed about before deciding actions.

exaby73 wrote this answer on 2022-11-24

Thanks for the information :) I'll label this issue for further discussion from the team. Thank you

More Details About Repo
Owner Name flutter
Repo Name flutter
Full Name flutter/flutter
Language Dart
Created Date 2015-03-06
Updated Date 2022-12-07
Star Count 147031
Watcher Count 3560
Fork Count 23915
Issue Count 11300

YOU MAY BE INTERESTED

Issue Title Created Date Updated Date