Skip to main content
Code Review

Return to Answer

added 66 characters in body
Source Link
Flambino
  • 33.3k
  • 2
  • 46
  • 90

If this is Rails, you've got the #underscore mixin from ActiveSupport at your disposal, which'll at least take care of the last bit.

As for the naming in general, I'm wondering what purpose the full names serve, if so much of them is just dropped. I'm not saying it's wrong, I'm just wondering. Especially as it seems like the transformations are one-way.

What I mean is that the class OrderByBestValueFilter becomes just best_value, effectively "throwing away" a lot of information in the name. If said information can be thrown away, was it even necessary in the first place?

As for the transformations being one-way, it's just another indication that the class name → criterion* string rule is a little informal. The string best_value could come from a class called BestValue, ByBestValue, BestValueFilter, or some other combination. Yet I'd imagine an OrderByXyzFilter is semantically different than a ByXyzFilter: The former takes a collection and sorts its elements without removing anything, while the latter takes a collection and removes elements not matching some predicate. Yet despite being very different, both would just end being called xyz. I just find that a little suspicious.

My thinking is also that you can side-step the entire thing by, for instance, putting your classes in a Filters module. Within that namespace, they could just be called BestValue and so forth. Or each class can declare its "criterion name" in some explicit way, e.g. as a class method.

Again, I don't know your code. I'm just thinking out loud.

As for the way you're doing things now, it's not too bad, though the using array subtraction is a bit much just to remove parts of a string. It could be made a little neater with a combined regexp:

name = self.class.name.split('::').last
name.gsub(/(^(Order)?(By)?|Filter$)/, '').gsub(/([a-z])([A-Z])/, '1円_2円').downcase

I've anchored matching to avoid something like ByFilterSizeFilter becoming just size.

*) it's criterion when it's singular; criteria is plural

If this is Rails, you've got the #underscore mixin from ActiveSupport at your disposal, which'll at least take care of the last bit.

As for the naming in general, I'm wondering what purpose the full names serve, if so much of them is just dropped. I'm not saying it's wrong, I'm just wondering. Especially as it seems like the transformations are one-way.

What I mean is that the class OrderByBestValueFilter becomes just best_value, effectively "throwing away" a lot of information in the name. If said information can be thrown away, was it even necessary in the first place?

As for the transformations being one-way, it's just another indication that the class name → criterion* string rule is a little informal. The string best_value could come from a class called BestValue, ByBestValue, BestValueFilter, or some other combination. Yet I'd imagine an OrderByXyzFilter is semantically different than a ByXyzFilter: The former takes a collection and sorts its elements without removing anything, while the latter takes a collection and removes elements not matching some predicate. Yet despite being very different, both would just end being called xyz. I just find that a little suspicious.

My thinking is also that you can side-step the entire thing by, for instance, putting your classes in a Filters module. Within that namespace, they could just be called BestValue and so forth. Or each class can declare its "criterion name" in some explicit way, e.g. as a class method.

Again, I don't know your code. I'm just thinking out loud.

As for the way you're doing things now, it's not too bad, though the . It could be made a little neater with a combined regexp:

name = self.class.name.split('::').last
name.gsub(/(^(Order)?(By)?|Filter$)/, '').gsub(/([a-z])([A-Z])/, '1円_2円').downcase

I've anchored matching to avoid something like ByFilterSizeFilter becoming just size.

*) it's criterion when it's singular; criteria is plural

If this is Rails, you've got the #underscore mixin from ActiveSupport at your disposal, which'll at least take care of the last bit.

As for the naming in general, I'm wondering what purpose the full names serve, if so much of them is just dropped. I'm not saying it's wrong, I'm just wondering. Especially as it seems like the transformations are one-way.

What I mean is that the class OrderByBestValueFilter becomes just best_value, effectively "throwing away" a lot of information in the name. If said information can be thrown away, was it even necessary in the first place?

As for the transformations being one-way, it's just another indication that the class name → criterion* string rule is a little informal. The string best_value could come from a class called BestValue, ByBestValue, BestValueFilter, or some other combination. Yet I'd imagine an OrderByXyzFilter is semantically different than a ByXyzFilter: The former takes a collection and sorts its elements without removing anything, while the latter takes a collection and removes elements not matching some predicate. Yet despite being very different, both would just end being called xyz. I just find that a little suspicious.

My thinking is also that you can side-step the entire thing by, for instance, putting your classes in a Filters module. Within that namespace, they could just be called BestValue and so forth. Or each class can declare its "criterion name" in some explicit way, e.g. as a class method.

Again, I don't know your code. I'm just thinking out loud.

As for the way you're doing things now, it's not too bad, though using array subtraction is a bit much just to remove parts of a string. It could be made a little neater with a combined regexp:

name = self.class.name.split('::').last
name.gsub(/(^(Order)?(By)?|Filter$)/, '').gsub(/([a-z])([A-Z])/, '1円_2円').downcase

I've anchored matching to avoid something like ByFilterSizeFilter becoming just size.

*) it's criterion when it's singular; criteria is plural

deleted 1 character in body
Source Link
Flambino
  • 33.3k
  • 2
  • 46
  • 90

If this is Rails, you've got the #underscore mixin from ActiveSupport at your disposal, which'll at least take care of the last bit.

As for the naming in general, I'm wondering what purpose the full names serve, if so much of them areis just dropped. I'm not saying it's wrong, I'm just wondering. Especially as it seems like the transformations are one-way.

What I mean is that the class OrderByBestValueFilter becomes just best_value, effectively "throwing away" a lot of information in the name. If said information can be thrown away, was it even necessary in the first place?

As for the transformations being one-way, it's just another indication that the class name → criterion* string rule is a little informal. The string best_value could come from a class called BestValue, ByBestValue, BestValueFilter, or some other combination. Yet I'd imagine an OrderByXyzFilter is semantically different than a ByXyzFilter: The former takes a collection and sorts its elements without removing anything, while the latter takes a collection and removes elements not matching some predicate. Yet despite being very different, both would just end being called xyz. I just find that a little suspicious.

My thinking is also that you can side-step the entire thing by, for instance, putting your classes in a Filters module. Within that namespace, they could just be called BestValue and so forth. Or each class can declare its "criterion name" in some explicit way, e.g. as a class method.

Again, I don't know your code. I'm just thinking out loud.

As for the way you're doing things now, it's not too bad, though the . It could be made a little neater with a combined regexp:

name = self.class.name.split('::').last
name.gsub(/(^(Order)?(By)?|Filter$)/, '').gsub(/([a-z])([A-Z])/, '1円_2円').downcase

I've anchored matching to avoid something like ByFilterSizeFilter becoming just size.

*) it's criterion when it's singular; criteria is plural

If this is Rails, you've got the #underscore mixin from ActiveSupport at your disposal, which'll at least take care of the last bit.

As for the naming in general, I'm wondering what purpose the full names serve, if so much of them are just dropped. I'm not saying it's wrong, I'm just wondering. Especially as it seems like the transformations are one-way.

What I mean is that the class OrderByBestValueFilter becomes just best_value, effectively "throwing away" a lot of information in the name. If said information can be thrown away, was it even necessary in the first place?

As for the transformations being one-way, it's just another indication that the class name → criterion* string rule is a little informal. The string best_value could come from a class called BestValue, ByBestValue, BestValueFilter, or some other combination. Yet I'd imagine an OrderByXyzFilter is semantically different than a ByXyzFilter: The former takes a collection and sorts its elements without removing anything, while the latter takes a collection and removes elements not matching some predicate. Yet despite being very different, both would just end being called xyz. I just find that a little suspicious.

My thinking is also that you can side-step the entire thing by, for instance, putting your classes in a Filters module. Within that namespace, they could just be called BestValue and so forth. Or each class can declare its "criterion name" in some explicit way, e.g. as a class method.

Again, I don't know your code. I'm just thinking out loud.

As for the way you're doing things now, it's not too bad, though the . It could be made a little neater with a combined regexp:

name = self.class.name.split('::').last
name.gsub(/(^(Order)?(By)?|Filter$)/, '').gsub(/([a-z])([A-Z])/, '1円_2円').downcase

I've anchored matching to avoid something like ByFilterSizeFilter becoming just size.

*) it's criterion when it's singular; criteria is plural

If this is Rails, you've got the #underscore mixin from ActiveSupport at your disposal, which'll at least take care of the last bit.

As for the naming in general, I'm wondering what purpose the full names serve, if so much of them is just dropped. I'm not saying it's wrong, I'm just wondering. Especially as it seems like the transformations are one-way.

What I mean is that the class OrderByBestValueFilter becomes just best_value, effectively "throwing away" a lot of information in the name. If said information can be thrown away, was it even necessary in the first place?

As for the transformations being one-way, it's just another indication that the class name → criterion* string rule is a little informal. The string best_value could come from a class called BestValue, ByBestValue, BestValueFilter, or some other combination. Yet I'd imagine an OrderByXyzFilter is semantically different than a ByXyzFilter: The former takes a collection and sorts its elements without removing anything, while the latter takes a collection and removes elements not matching some predicate. Yet despite being very different, both would just end being called xyz. I just find that a little suspicious.

My thinking is also that you can side-step the entire thing by, for instance, putting your classes in a Filters module. Within that namespace, they could just be called BestValue and so forth. Or each class can declare its "criterion name" in some explicit way, e.g. as a class method.

Again, I don't know your code. I'm just thinking out loud.

As for the way you're doing things now, it's not too bad, though the . It could be made a little neater with a combined regexp:

name = self.class.name.split('::').last
name.gsub(/(^(Order)?(By)?|Filter$)/, '').gsub(/([a-z])([A-Z])/, '1円_2円').downcase

I've anchored matching to avoid something like ByFilterSizeFilter becoming just size.

*) it's criterion when it's singular; criteria is plural

Source Link
Flambino
  • 33.3k
  • 2
  • 46
  • 90

If this is Rails, you've got the #underscore mixin from ActiveSupport at your disposal, which'll at least take care of the last bit.

As for the naming in general, I'm wondering what purpose the full names serve, if so much of them are just dropped. I'm not saying it's wrong, I'm just wondering. Especially as it seems like the transformations are one-way.

What I mean is that the class OrderByBestValueFilter becomes just best_value, effectively "throwing away" a lot of information in the name. If said information can be thrown away, was it even necessary in the first place?

As for the transformations being one-way, it's just another indication that the class name → criterion* string rule is a little informal. The string best_value could come from a class called BestValue, ByBestValue, BestValueFilter, or some other combination. Yet I'd imagine an OrderByXyzFilter is semantically different than a ByXyzFilter: The former takes a collection and sorts its elements without removing anything, while the latter takes a collection and removes elements not matching some predicate. Yet despite being very different, both would just end being called xyz. I just find that a little suspicious.

My thinking is also that you can side-step the entire thing by, for instance, putting your classes in a Filters module. Within that namespace, they could just be called BestValue and so forth. Or each class can declare its "criterion name" in some explicit way, e.g. as a class method.

Again, I don't know your code. I'm just thinking out loud.

As for the way you're doing things now, it's not too bad, though the . It could be made a little neater with a combined regexp:

name = self.class.name.split('::').last
name.gsub(/(^(Order)?(By)?|Filter$)/, '').gsub(/([a-z])([A-Z])/, '1円_2円').downcase

I've anchored matching to avoid something like ByFilterSizeFilter becoming just size.

*) it's criterion when it's singular; criteria is plural

lang-rb

AltStyle によって変換されたページ (->オリジナル) /