37
An alternative route is to use string.replace() and Regular Expressions to strip everything but the vowels from the string. Then count the length of the resulting string. This avoids iteration altogether.
const vowelCount = s => s.replace(/[^aeiou]/gi, '').length
console.log(vowelCount('The quick brown fox jumps over the lazy dog'))
The regex ...
35
Variables
I understand why you have the Range.this.end and Range.this.start to avoid confusion about where those variables come from... If you need the Range.this as part of the teaching exercise, then sure. Otherwise, I would recommend three things....
add range as a prefix, even though it is slightly redundant
Make them final...
one variable per line... (...
31
Why didn't Microsoft include this in the .NET framework?
Probably because it can be realized in one line with Select:
source.Select(x => x.Equals(oldValue) ? newValue : x)
or with 2 lines as extension method if the function is needed frequently.
public static IEnumerable<T> Replace<T>(this IEnumerable<T> source, T oldValue, T ...
30
Your iterator is missing a few important details:
You should provide the pre- and post-increment operators (++it and it++). Currently, you only have the pre-increment version.
It might also be good to provide the -> operator, since some people prefer the it->something syntax over the (*it).something one.
The comparison and dereference operators should ...
26
Two things:
I think you have missed the native implementation. Are you intentionally re-inventing the wheel? Streams have an iterator() method.
Note that streams cannot be reversed, so, while you can create an iterator once, from the stream, you cannot create a second iterator.... In other words, you cannot loop more than once through your iterable.
...
26
I'm just wondering if there's an easier way to solve this problem without the big blocky code in the if statement.
Well, you could put all of those in an array:
const vowels = ['A', 'E', 'I', 'O', 'U', 'a', 'e', 'i', 'o', 'u'];
And then the if condition can be simplified using Array.prototype.includes():
if( vowels.includes(v)) {
The fact that I have ...
25
If you're targeting Java 8, then you should make an IntStream and a PrimitiveIterator.OfInt instead. You can do more interesting manipulations with an IntStream, and the primitive types would be more efficient than the boxed types.
In fact, there is already an IntStream.range(startInclusive, endExclusive) function. You could simply .map() that to another ...
24
Nothing major (just some personal opinion ones):
Consistent Spacing
infix_ostream_iterator(ostream_type& s)
: os(&s),delimiter(0), first_elem(true)
{} // ^^^ No Space ^^^Trailing space
Consistent type Naming
// Here we have & on the left
infix_ostream_iterator<T,charT,traits>& operator=(T const &item)
// ...
24
I cannot comment about the speed because I didn't benchmark it but there are two obvious improvements.
add an IEqualityComparer<T> as a third parameter so that you can use a custom one if necessary (e.g. own typs)
use foreach instead of the enumerator
Example:
public static IEnumerable<T> Replace<T>(this IEnumerable<T> source, T ...
16
You should not need a special class for this. Method references can be cast to functional interfaces. In this particular case, however, remember that the stream returned by Files::lines needs to be closed to prevent leaks:
try (Stream s = Files.lines(...)) {
for (String v : (Iterable<String>) s::iterator) {
...
}
}
15
The only thing at all that I can criticise in the code is the inconsistent placement of whitespace between infix operators and in pointer / reference declarations.
class charT=char,
public std::iterator<std::output_iterator_tag,void,void,void,void>
… etc., missing spaces.
: os(&s),delimiter(0), first_elem(true)
… etc., inconsistent use of ...
15
I'm not going to say I'd use this, nor that I wouldn't.
I think it's a clever1 use of generics, and turns C# enums into something else that could possibly be called JavaIshEnum; being an actual class, you significantly blur the line between classes and enums, but you also effectively work around the annoying absence of a generic type constraint in the C# ...
15
You can use the Linq Take to accomplish the same thing
public static class IEnumerableTCountExtensions
{
public static bool CountAtLeast<T>(this IEnumerable<T> items, Func<T, bool> predicate, int required)
{
var count = items.Where(predicate).Take(required).Count();
return count == required;
}
public static ...
15
First, let's look at the naive implementation using sets:
def diff_set(before, after):
b, a = set(before), set(after)
return list(a - b), list(b - a), list(a & b)
This is (so much more) readable and understandable than your code. The question is, is this significantly slower than your code?
Here are timings of your three example cases:
...
14
I have not much to say. Your code reads quite good, which is rather pleasant. Here are a few tidbits though:
typedef
If you are willing to write modern code, you should consider dropping typedef and using using everywhere instead. It helps to be consistent between regular alias and alias template. Moreover, the = symbol help to visually split the new name ...
14
Minor point.
The step is zero
is an odd error message. Wouldn't it be better to actual explain the problem, ie.
The step cannot be zero
The user (hopefully) knows what paramaters they've supplied, what you need to tell them is that the parameter is invalid.
As @200_success pointed out in a comment, Python follows this with its own error messages: ...
13
The requirements for a forward iterator are:
Forward Iterator
It is a refinement of:
Input Iterator
Output Iterator
Trivial Iterator
Equity Comparable
Default Constructable
Assignable
If you read through all those specs you must define these:
Preincrement
Postincrement
Dereference (Read/Write)
Default Constructable
Copy Constructable
Assignment operator
...
13
Issues
public void Push(T item)
{
_stackList.Insert(0, item);
}
The hurts the stack list.
It would be much easier if you just added new items at the end of the list. The Insert needs to rebuild the entire list:
This method is an O(n) operation, where n is Count.
For an addition this is a real bottleneck.
The same applies to the Pop method which ...
13
Algorithm
What you have implemented is called (linear) insertion sort. It is very old, sorry your method is not new or novel ;) It has average case O(n^2) time complexity which is basically unusable for anything with more than a few thousand/tens of thousands elements. I recommend that you study the classic algorithms: Bubble Sort, (Cocktail) Shaker Sort, ...
13
There are no docstrings. What do these functions do? What arguments do they take? What do they return?
Decorators are normally named after the effect they have on the decorated function. For example @functools.lru_cache adds an LRU cache to a function; @unittest.skip causes a test case to be skipped, and so on.
The effect of @from_iterable is that it ...
12
There are a few serious things that haven't been brought up by the other two answers yet.
Dereferencing issues
Let's say I am zipping a vector<T> and a vector<U>. Your iterator's current member will have type std::tuple<std::vector<T>::iterator, std::vector<U>::iterator> and its value_type will be std::tuple<T, U> and ...
12
A couple of real issues (rather than the imaginary ones previously pointed out):
Your code does not fully implement the requirements of iterator.
You have not implemented the operator-> as required by the iterator concept.
operator* should return by reference.
This is because *x = t is a requirement of iterator and without the reference this will not ...
12
Overall, it's pretty good code to learn from.
Functionality
I like that you've used the inclusive-exclusive convention for the lower and upper bounds, respectively. The rationale for that design would be an interesting discussion topic.
I suggest adding a second constructor for convenience:
public Range(int end) {
this(0, end);
}
There should ...
12
After reading the problem description and before reading your code, I implemented something that was very similar to what you already have, which means that your approach to the problem is perfectly fine.
Spliterator - of unknown size
Instead of using
Spliterators.spliterator(it, Long.MAX_VALUE, Spliterator.ORDERED)
You should use
Spliterators....
12
Naming
First of, what you have created is not an iterator so naming it as such is misleading. I would recommend a name but I actually think that you should redesign your class and the name I would recommend wouldn't be fit for the redesigned class so I'll wait with that until the end.
Design
In C++ as opposed to Java, when you assign an object you get a ...
12
There are so many different solutions already that I won't post another. But there is something else that bothers me, the names of the extensions. I find they are too confusing. Expecially Above and Below. They are not very technical (at leat in this context).
I think they should be called the same way we call the conditions they are based on and that are (...
12
Use delete[]
You are allocating an array of objects with new[], but attempting to delete them with delete, which results in undefined behavior. You should delete the array with delete[] to ensure that your code functions properly. Do this in both Vector<T>::~Vector() and Vector<T>::insert().
Implement your Iterators as pointers under-the-hood
...
12
The range-based for loop
This works via the begin() and end() functions.
So for any class X if there are stand alone functions begin(X&) and end(X&) the range based for will work. Normally this will default to the std::begin() and std::end() but if you have written explicit versions of these functions in the same namespace as X these will be used.
...
12
I have some ideas about how you might be able to improve your program.
Avoid problems
Rather than trying to deal with the problem for every instruction, one approach is avoiding it entirely. One way to do that is to simply append a number of bytes to the end of the vector. If the maximum bytes for an instruction is \$n\$, then append \$n-1\$ bytes to the ...
11
Empty Lists
if (list.isEmpty()) {
throw new IllegalArgumentException("list cannot be empty");
}
Yuck ... you should be able to handle an empty list more gracefully than that. Creating a special EmptyCircularListCursor instance perhaps.
Random Access
I like that you differentiate between RandomAccess and other lists. This is a good thing.
...
Only top voted, non community-wiki answers of a minimum length are eligible
