30

I think your code is perfectly understandable as it is. Here's an idea though. (In case it is not clear: the suggestions here are more for amusement and edification than a serious suggestion. The original procedural code is just fine, but it is interesting to see how it might be done in a functional style.) var funcs = new List<Func<BookInfo, object&...


27

OK, went through the code and gave this a bit of thought over the past couple of days. As far as the implementation goes, I don't see a whole lot that I would change (that you didn't identify in the answer above) other than a couple nit-picky things. First, is the use of the this variable identifier. I couldn't find anything that justifies the naming and ...


25

This is super simple; use an array and a for-loop: int rowIndex = 2; foreach (BookInfo book in books) { var columns = new object[]{ book.BookId, book.Book, book.System, book.Age, book.StartDate, book.Pages, }; for (int j = 0; j < columns.Length; j++) { excelExport.SetCell(j + 1, ...


22

Use SelectMany. As in, allPersons.SelectMany(s => s);. This flattens the sequence for you.


21

This is called a doubly linked list. List<T> is basically a wrapper around an array. The only operations where you can hope to be faster are insertions and deletions from the middle of the list. Using Marshal unless you absolutely have to is a bad idea, if not a plain crazy one. This code has memory leaks (how is that destructor meant to release all ...


20

I would advise against this approach, you are correct in the fact that it is simple, however it's not foolproof. There is nothing actually ensuring that someone doesn't just call .Add() on the collection bypassing your extension method which allows for difficult to find bugs/race conditions. Don't forget that all Monitor.Enter does is ensure that if ...


20

Public Function ToString() As String 'Returns a string that represents the current List object. ToString = StringFormat("{0}<{1}>", TypeName(Me), _ Coalesce(this.ItemTypeName, "Variant")) End Function This means the string representation of this List is "List<Variant>" when this.ItemTypeName is empty ...


16

Is there a common Java library that serves a similar purpose, or am I reinventing-the-wheel? As far as I know the JRE doesn't offer this out of the box. Are the semantics for null keys and zero counts reasonable? Would it be typical to be counting nulls? I guess it's nice that it's possible, even though it opens the possibility for exceptions. ...


15

Please fix your indentation all over the header. It should be consistent with everything else. You don't need the return 0 at the end of main(). Reaching this point implies successful termination, and the compiler will just insert it for you. In common stack implementations, pop() is void (merely pops off the top element). This is useful as it'll be much ...


15

First things first, I feel the biggest problem about your code is that your comments lie. /** * i> Read a List Input, * ii> Call a Function with maximum of 99 elements in one iteration Eg:(0..98; 99..198; 199..297; .....) * * */ public class OptimizeReadingList There is no List to be seen, and I am not sure what is meant by 'call a function'....


14

First of all, I will start with one of the most common remarks: please, do not use using namespace std;. It is especially bad if you write it in a header file since it leads to namespace pollution and name clashes. Instead of a method named traverse, it would be better to overload operator<< to print your list. Here is how you could adapt your ...


14

There are a few improvements I can see, but not really related to performance (though it may run a bit faster anyway). If the files are small (less than 10Megabytes or so), then the relative performance of different approaches are slight (obviously, you can bork your code, but the IO part will remain about the same). So, it comes down to readability. For ...


14

Bugs Your code doesn't actually do what it says it will do. The problems are here: for(int i = 0,ele = (read_max_size-1); i<input_list_size ; ){ printElements(i,ele); // Increment Operations for the next elements(maybe another 99 or less than 99) i=(ele+1); if((ele+read_max_size > input_list_size)){ ...


13

Let's take a look at naming and style. public class OptimizeReadingList { That sounds like a function, not like a class. // Add Elements to the Array for(int j =0;j<input_list_size;j++){ a[j]=j+1; } Comments should explain why, not how. Also, your spacing is inconsistent. The following would be more readable: ...


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

Sorting the source is not specified as a requirement, so it seems that std::partition would do the necessary job more efficiently. As a perk benefit, std::partition returns the partition point, eliminating the need to std::find_if. If you still want to follow the sorting path, consider std::lower_bound instead of find_if.


12

[...]I decided that it would be fine to have something like an arbitrary-keyed map, that would require the key to be stored in the object (which allows to use the object's name as a key for instance). To restate your problem: you want the add behavior of a set (i.e., you add the object to the collection without any other information), but you want the ...


11

You can use LINQ's Zip method. As usual, it's a bit slower than manually written code, but unless this is in a hot spot it rarely matters. The cost of Console.WriteLine vastly exceeds the cost of LINQ. foreach(var pair in collection1.Zip(collection2, Tuple.Create)) { Console.WriteLine("{0}, {1}", pair.Item1, pair.Item2); } You can replace Tuple.Create ...


11

Code smell: you are doing the same thing over and over again, with different instances of the same variable type. if(idValue!=null) { entireList=idValue.sortedCopy(entireList); } else if(valueId!=null) { entireList=valueId.sortedCopy(entireList); }else if(id!=null) { ...


10

If you look for performance, you'd better rely on the framework dictionary. You can use the Dictionary<string, List<object>> to do what you need. When you add a new key and value, look if the key already exists. If so, add the value to the array and don't add the key.


10

Well, you could just call the toArray method of your list to transform the list into an array. Or even better, you could make the table model use a List<Object[]> rather than an Object[][], which would avoid the unnecessary conversion.


10

First, use generics properly. Second, why do you track the cache size if when you need it, you get a hardcoded 99? I will assume that was some sort of mistake. Third, if you really want to annoy most truely professional java developers and course instructors (i.e, not these that write Java code as if they were programming in C#, C++ or Pascal), prefix your ...


10

I don't really have time to read through the whole code and the existing reviews now but as far I see no-one mentioned Guava's Multiset and its AtomicLongMap yet. I've been able to change Counter to HashMultiset in the test methods with very few issues while the bar remained green: Multiset does not store keys (elements) with zero occurrences. (...


10

A few things that @rolfl didn't mention explicitly: public static Double integerFromString(String str) { return Double.valueOf(str.substring(0, str.length())); } This code alone can be significantly improved. @rolfl is right that it has a confusing name as it returns a Double. When you use str.substring(0, str.length()) you'll get back exactly the ...


10

Design I think you need to break that class in two. Having instance members on a static class is pretty confusing and bug-prone. I'd suggest Enumerable to be the static class, with these members (notice source being preferred over collectionObject, and explicit ByRef modifiers and Variant types): Function Contains(ByRef source As Variant, ByVal value As ...


10

There are several thread safety issues : if size(), remove() and removeAll() need to wait(), they will throw an IllegalMonitorStateException since they do not hold the lock on this at the time they call wait(); for that same reason, they may see stale values for isUpdating if size() would be interrupted while waiting (supposing that gets fixed) its return ...


10

There is a whole bunch of throws UnsupportedOperationException in your code. More specifically, everything that has to do with specific indexes seems to be unsupported. All this makes me wonder: Should you really implement the List interface? In my opinion, you should not. Because essentially, those List-specific methods are not supported. I think you ...


10

Note that java.util.List and java.util.Set, in addition to imposing various requirements that the compiler is capable of enforcing, also impose various requirements via Javadoc. Your class does not satisfy those requirements, and in fact it is not possible for a single class to satisfy the requirements of both interfaces. In particular, take a look at the ...


10

This would be much easier to review if there were more useful comments (saying "forward declarations" in front of a bunch of forward declarations is not all that useful) or - more importantly - some actual code showing how these classes are intended to be used in practice. Whether or a class (hierarchy) is well designed depends almost entirely on how it's ...


10

No and no. This is a very inefficient way to reverse, for example, a dynamic array based list (it's a different story with a linked list). With a dynamic array based (e.g. ArrayList) The remove(0) operation will require moving every other element in the list (linear in the length), so the whole method is quadratic in the total length of the list. It also ...


Only top voted, non community-wiki answers of a minimum length are eligible