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

Match.start, groupCount, and end should be properties, not methods #2128

Closed
DartBot opened this issue Mar 13, 2012 · 3 comments
Closed

Match.start, groupCount, and end should be properties, not methods #2128

DartBot opened this issue Mar 13, 2012 · 3 comments
Labels
area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries.

Comments

@DartBot
Copy link

DartBot commented Mar 13, 2012

This issue was originally filed by [email protected]


What steps will reproduce the problem?

  1. Use RegExp, get a Match
  2. Try to find out where it starts and ends:

main() {
  Match m = new RegExp("h").firstMatch("hello");
  print(m.start);
}

What is the expected output? What do you see instead?

Expected: 0 Got: Object

It is not conceptually a method, as I am not telling the Match to start something.

What version of the product are you using? On what operating system?

DartPad, Dart Editor

Please provide any additional information below.

From http://www.dartlang.org/articles/style-guide/

DO use a getter for operations that conceptually access a property.

If the name of the method starts with get or is an adjective like visible or empty that's a sign you're better off using a getter. More specifically, a getter should:

Not take any arguments.
Return a value.
Be side-effect free. Invoking a getter shouldn't change any externally-visible state (caching internally or lazy initialization is OK). Invoking the same getter repeatedly should return the same value unless the object is explicitly changed between calls.
Be fast. Users expect expressions like foo.bar to execute quickly.

You should also consider adopting naming conventions for distinguishing properties and methods while you can still get away with renaming APIs and breaking everyone's code; for example the .NET style guide states that properties should generally be nouns and methods should start with verbs. (http://msdn.microsoft.com/en-us/library/bzwdh01d(v=vs.71).aspx) This not only helps users distinguish them but it also helps writers think about which they are writing.

As an example, hashCode() could either be made a property, or, if the point of making it a method is to let the user know it does something, then its name should be changed to something that suggests that it does something, like getHashCode() or even computeHashCode(). Because properties are a feature of this language, methods should not be named like they are properties; that just causes people to closurize them all the time. Methods get these kinds of names in languages that don't have properties; fight these habits.

@madsager
Copy link
Contributor

Added Area-Library, Triaged labels.

@DartBot
Copy link
Author

DartBot commented Mar 13, 2012

This comment was originally written by [email protected]


I've reviewed all of dart:core, and here are the names I think should be reviewed for consistency.

Methods that should clearly be properties:

Match.start
Match.end
Match.groupCount

Other methods with nouns for names, many arguably should be properties:

Clock.frequency
Clock.now -- this is not a property, and while you could make it "getCurrentTime", "now" is common enough that I'm not sure it should change.
Hashable.hashCode
Iterable.iterator
Iterator.next
List.last
num.floor -- This is a noun, but I think of "floor" as a mathematical operation that you are asking it to do, like sin or abs, so I'm inclined to say this name is fine.
Queue.first
Queue.last
Stopwatch.frequency
Stopwatch.elapsed -- not a noun, definitely not a verb, best understood as short for "elapsed time" which is a noun
String.charCodes

Methods that could have been given nouns for names, but were not:

Map.getKeys
Map.getValues

Boolean methods/properties tend to ask a question and not be nouns, so more naming rules will need to be considered in this case. For these I don't see a discernable pattern to what you make methods versus properties:

Collection.isEmpty() (also Map, String, StringBuffer)
Date.isLocalTime()
Date.isUTC()
Future.hasValue
Future.isComplete
Iterator.hasNext()
num.isEven() (and other is methods on num)
TimeZone.isUTC

@justinfagnani
Copy link
Contributor

Looks like all of these have been fixed, or are obsolete.


Added Fixed label.

@DartBot DartBot added Type-Defect area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. labels Apr 17, 2013
copybara-service bot pushed a commit that referenced this issue Jun 5, 2023
…est, tools, webdev

Revisions updated by `dart tools/rev_sdk_deps.dart`.

async (https://github.com/dart-lang/async/compare/96c29d0..a506993):
  a506993  2023-06-01  Kevin Moore  Update testing SDK in CI (#245)
  9be3fc0  2023-06-01  Kevin Moore  Require Dart 2.19, use latest team lints (#244)

dartdoc (https://github.com/dart-lang/dartdoc/compare/39fe1a8..0c8feac):
  0c8feac6  2023-06-04  Sam Rawlins  Sort enum_test.dart (#3428)

ffi (https://github.com/dart-lang/ffi/compare/7f4acbd..f582ca0):
  f582ca0  2023-05-17  Daco Harkes  Rename `master` branch to `main` (#197)
  604451d  2023-05-16  Devon Carew  blast_repo fixes (#195)

http (https://github.com/dart-lang/http/compare/8834aec..5312366):
  5312366  2023-06-02  Brian Quinlan  Reland "support the nsurl session web socket api" (#950)

leak_tracker (https://github.com/dart-lang/leak_tracker/compare/cbbdeca..f17da61):
  f17da61  2023-06-02  Polina Cherkasova  Add constructor for retaining path. (#72)
  b70e538  2023-06-01  Polina Cherkasova  Fix connection issue. (#70)
  a80f253  2023-06-01  dependabot[bot]  Bump actions/checkout from 3.3.0 to 3.5.2 (#71)

lints (https://github.com/dart-lang/lints/compare/edc28ed..fc74ce0):
  fc74ce0  2023-06-02  Devon Carew  update the readme to clarify the package's goals (#130)
  4e4c18f  2023-06-02  Devon Carew  mv analysis_options file (#129)

mockito (https://github.com/dart-lang/mockito/compare/924f65c..cca4858):
  cca4858  2023-06-02  Ilya Yanok  Fix the unresolved types if used as ignored type-alias arguments
  8652886  2023-06-01  Googler  Prepare for NamedType breaking change in the analyzer.

test (https://github.com/dart-lang/test/compare/3276921..7a6c98d):
  7a6c98d0  2023-06-01  Konstantin Scheglov  Require analyzer 5.12.0, refactor InstanceCreationExpression type name extraction. (#2015)
  23bd4159  2023-06-01  Nate Bosch  Use switch expressions for switch/return pattern (#2027)
  06bdbb65  2023-06-01  Nate Bosch  Make State and Result enums (#2028)
  11805dc5  2023-06-01  Nate Bosch  Migrate to Dart 3 (#2024)

tools (https://github.com/dart-lang/tools/compare/389925f..8d6e8b8):
  8d6e8b8  2023-06-01  Kevin Moore  unified_analytics and graphs: cleanup lints, bump pkg deps (#108)

webdev (https://github.com/dart-lang/webdev/compare/f565d7f..b10d62b):
  b10d62b8  2023-06-02  Anna Gringauze  Support using scope in evaluateInFrame (#2122)
  c0300ce6  2023-06-02  Elliott Brooks  Included requested-by header in `ProxyServerAssetReader` (#2129)
  57699563  2023-06-01  dependabot[bot]  Bump actions/labeler from 4.0.3 to 4.0.4 (#2128)

Change-Id: Ia6114a0cbbe789f13e2c42399f53ff0e4d29d74a
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/307502
Auto-Submit: Devon Carew <[email protected]>
Reviewed-by: Konstantin Shcheglov <[email protected]>
Commit-Queue: Konstantin Shcheglov <[email protected]>
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries.
Projects
None yet
Development

No branches or pull requests

3 participants