Skip to content

Commit 713900a

Browse files
committed
Strip out unnecessary comments
1 parent 55b999f commit 713900a

File tree

1 file changed

+38
-54
lines changed

1 file changed

+38
-54
lines changed

lib/analyzers/two_fer/analyze.rb

Lines changed: 38 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -17,44 +17,17 @@ module TwoFer
1717
class Analyze < ExerciseAnalyzer
1818
include Mandate
1919

20+
# Note that all "check_...!" methods exit this method if the solution
21+
# is approved or disapproved, so each step is only called if the
22+
# previous one has not resolved what to do.
23+
2024
def analyze!
2125
#target_method.pry
22-
23-
# Note that all "check_...!" methods exit this method if the solution
24-
# is approved or disapproved, so each step is only called if the
25-
# previous one has not resolved what to do.
26-
27-
# Firstly we want to check that the structure of this
28-
# solution is correct and that there is nothing structural
29-
# stopping it from passing the tests
3026
check_structure!
31-
32-
# Now we want to ensure that the method signature
33-
# is sane and that it has valid arguments
3427
check_method_signature!
35-
36-
# There is one optimal solution for two-fer which needs
37-
# no comments and can just be approved. If we have it, then
38-
# let's just acknowledge it and get out of here. We also often see
39-
# solutions that are correct but use different # string concatenation
40-
# options (e.g. string#+, String.format, etc). We'll approve these but
41-
# want to leave a comment that introduces them to string interpolation
42-
# in case they don't know about it.
4328
check_for_single_line_solution!
44-
45-
# The most common error in twofer is people using conditionals
46-
# to check where the value passed in is nil, rather than using a default
47-
# value. We want to check for conditionals and tell the user about the
48-
# default parameter if we see one.
4929
check_for_conditional_on_default_argument!
50-
51-
# Sometimes, rather than setting a variable, people reassign the input param e.g.
52-
# name ||= "you"
5330
check_for_reassigned_parameter!
54-
55-
# Sometimes people specify the names (if name == "Alice" ...). If we
56-
# do this, suggest using string interpolation to make us of the
57-
# parameter, rather than using a conditional on it.
5831
# check_for_names!
5932

6033
# We don't have any idea about this solution, so let's refer it to a
@@ -65,39 +38,43 @@ def analyze!
6538
# ###
6639
# Analysis functions
6740
# ###
41+
42+
# Firstly we want to check that the structure of this
43+
# solution is correct and that there is nothing structural
44+
# stopping it from passing the tests
6845
def check_structure!
69-
# First we check that there is a two-fer class or module
70-
# and that it contains a method called two-fer
7146
disapprove!(:no_module) unless solution.has_target_module?
7247
disapprove!(:no_method) unless solution.has_target_method?
7348
end
7449

50+
# Then we we want to ensure that the method signature
51+
# is sane and that it has valid arguments
7552
def check_method_signature!
76-
# If there is no parameter or it doesn't have a default value,
77-
# then this solution won't pass the tests.
7853
disapprove!(:missing_default_param) unless solution.has_one_parameter?
79-
80-
# If they provide a splat, the tests can pass but we
81-
# should suggest they use a real parameter
8254
disapprove!(:splat_args, solution.first_parameter_name) if solution.uses_splat_args?
83-
84-
# If they don't provide an optional argument the tests will fail
8555
disapprove!(:missing_default_param) unless solution.first_paramater_has_default_value?
8656
end
8757

58+
# There is one optimal solution for two-fer which needs
59+
# no comments and can just be approved. If we have it, then
60+
# let's just acknowledge it and get out of here. We also often see
61+
# solutions that are correct but use different # string concatenation
62+
# options (e.g. string#+, String.format, etc). We'll approve these but
63+
# want to leave a comment that introduces them to string interpolation
64+
# in case they don't know about it.
65+
# The optional solution looks like this:
66+
#
67+
# def self.two_fer(name="you")
68+
# "One for #{name}, one for me."
69+
# end
70+
71+
# The default argument must be 'you', and it must just be a single
72+
# statement using interpolation. Other solutions might be approved
73+
# but this is the only one that we would approve without comment.
8874
def check_for_single_line_solution!
8975
return unless solution.default_argument_is_optimal?
9076
return unless solution.one_line_solution?
9177

92-
# The optional solution looks like this:
93-
#
94-
# def self.two_fer(name="you")
95-
# "One for #{name}, one for me."
96-
# end
97-
98-
# The default argument must be 'you', and it must just be a single
99-
# statement using interpolation. Other solutions might be approved
100-
# but this is the only one that we would approve without comment.
10178
if solution.uses_string_interpolation?
10279
if solution.string_interpolation_is_correct?
10380
approve_if_implicit_return!
@@ -106,15 +83,12 @@ def check_for_single_line_solution!
10683
end
10784
end
10885

109-
# In the case of:
11086
# "One for " + name + ", one for me."
11187
approve_if_implicit_return!(:string_building) if solution.uses_string_building?
11288

113-
# In the case of:
11489
# format("One for %s, one for me.", name)
11590
approve_if_implicit_return!(:kernel_format) if solution.uses_kernel_format?
11691

117-
# In the case of:
11892
# "One for %s, one for me." % name
11993
approve_if_implicit_return!(:string_format) if solution.uses_string_format?
12094

@@ -123,8 +97,11 @@ def check_for_single_line_solution!
12397
return refer_to_mentor!
12498
end
12599

100+
# The most common error in twofer is people using conditionals
101+
# to check where the value passed in is nil, rather than using a default
102+
# value. We want to check for conditionals and tell the user about the
103+
# default parameter if we see one.
126104
def check_for_conditional_on_default_argument!
127-
# If there are no if statements then we can safely get out of here.
128105
return unless solution.has_any_if_statements?
129106

130107
# If there is more than one statement, then let's refer this to a mentor
@@ -139,8 +116,9 @@ def check_for_conditional_on_default_argument!
139116
refer_to_mentor!
140117
end
141118

119+
# Sometimes, rather than setting a variable, people reassign the input param e.g.
120+
# name ||= "you"
142121
def check_for_reassigned_parameter!
143-
# If there are no reassignments then we can safely get out of here.
144122
return unless solution.reassigns_parameter?
145123

146124
# If there is more than one statement, then let's refer this to a mentor
@@ -155,6 +133,12 @@ def check_for_reassigned_parameter!
155133
refer_to_mentor!
156134
end
157135

136+
# Sometimes people specify the names (if name == "Alice" ...). If we
137+
# do this, suggest using string interpolation to make us of the
138+
# parameter, rather than using a conditional on it.
139+
def check_for_names!
140+
end
141+
158142
# ###
159143
# Flow helpers
160144
#

0 commit comments

Comments
 (0)