Reputation: 1002 Grandmaster
- Active Posts:
- 975 (0.5 per day)
- 30-September 10
- Profile Views:
- Last Active:
- Nov 29 2015 03:00 PM
- OS Preference:
- Favorite Browser:
- Favorite Processor:
- Favorite Gaming Platform:
- Your Car:
- Who Cares
- Dream Kudos:
- Expert In:
Posts I've Made
Posted 28 Nov 2015The Object.GetType() method returns the actual runtime type of the instance it is called on, and so it will return the specific type of the form instance, even if that instance has been upcasted to the more general Form type.
Hence, the code can be simplified slightly:
Public Class MyButton Inherits Button Protected Overrides Sub onclick(ByVal e As System.EventArgs) MyBase.onclick(e) Dim TheForm As Form = CType(Activator.CreateInstance(Me.FindForm().GetType()), Form) TheForm.Show() End Sub End Class
Posted 22 Nov 2015You could improve your code by devising a way to keep track of how many times you've seen each number. A good way to do this is using an array of integers, where each slot in the array holds the number of times you've seen the number designated by the index of the slot. For example, the slot at index 0 would hold how many times you've seen the number 0.
This array can then be used as a lookup table when deciding whether or not to add each randomly generated number to your list of numbers. This is the simplest possible form of a data structure known as a hash table, which is a data structure that is commonly used when designing algorithms, because each lookup is generally very fast.
An example of how you could use this technique is given in the spoiler below.
Alternatively, since you say each number must occur two times, you could simply loop from 0 to the highest number, adding each number to the list twice as you go. Once all the numbers have been added, you could shuffle the list of numbers.
I personally prefer the second approach, as it avoids the situation where the random number generator pointlessly generates numbers that have already occurred the maximum number of times. Also, it is more efficient in terms of the memory used. Finally, I think it is slightly more natural and easy to read and understand, because it makes the intent of the code clearer.
Posted 29 Sep 2015Although it may not have much value in this case, I thought I'd mention that empty collections/arrays can be generally useful when used to avoid null checks.
For example, it is preferable to return an empty collection instead of null from a method, at least when the null would have no special significance beyond representing the absence of collection elements.
Doing this means callers of the method can loop through the returned collection without having to check for null, for example. In addition, it helps keep code clean and provides a more consistent (and therefore usable) API. Furthermore, it reduces the chances of getting a NullReferenceException, which can be useful (as long as you don't hide a bug/problem by avoiding the exception) because they can be tedious to debug.
This idea is an example of the Null Object design pattern.
Posted 1 Oct 2014The problem is that you are clearing the list in your RefreshAll() method with this line:
and you call RefreshAll() immediately after creating a new vertex in Button1_Click.
You have to remember how instances of classes (like List(Of T)) are passed. When you create a new vertex and pass it tempConnections, you are passing to the vertex a reference to the list referenced by tempConnections, meaning both tempConnections and the new vertex will refer to the exactly the same list object.
To avoid this, you would need to copy all the elements from the tempConnections list into a new, different list object in the new vertex.
The easiest way to achieve this would be to change this line in the Vertex class's constructor:
Connections = pConnections
Connections = New List(Of Vertex)(pConnections)
Posted 19 Jul 2014Since you stated that your goal is to build good habits, I'm going to use some of the points I make about your specific code to bring to light some more general software development good practices that I feel are often neglected in many initial teachings, thereby allowing bad habits to develop.
Note that you probably won't have covered a lot of the language features I mention here, so you can't possibly beat yourself up for not using them in your code. Hopefully though, as you get to some of the topics I mention, you will remember some of the points I make. At the very least, they should give you a few things to think about and look into.
Also note that many of the things I mention are only guidelines, not hard rules to be blindly applied to every situation.
Finally, note that I am assuming that your method is meant to allow the reading of a subset of the lines in the file, not necessarily all the lines in the file. If you intend to always read all the lines in the file, you don't need the linesOfTextInDoc parameter or the line counting code.
Taking into account that you are new, your code looks very good. Many of the things I am going to mention are merely enhancements and things to be aware of, rather than things you are doing wrong.
However, there are two tangible problems that you should definitely fix...
1) If a negative number is passed to the method, the method will throw an OverflowException on line 6 because an array cannot be initialized with a negative size. If that exception is not handled by your code, it will crash the program.
Now, the critical point here is not so much that the exception will potentially crash the program. If the code that calls this method gives it a negative number, crashing is a reasonable thing to do because, clearly, the code that calls the method is broken and needs fixing because asking to read a negative number of lines isn't reasonable.
The problem with your method is really the type of exception that is being thrown. An OverflowException doesn't clearly describe what the problem was, and so it makes debugging that bit more difficult.
A better thing to do would be to check for a negative number at the start of the method, and throw an ArgumentOutOfRangeException, as so:
Now if a negative number is passed in, you immediately get an error telling you exactly what you did wrong, and where your did it wrong.
Points to take away
[*] Validate incoming arguments to methods, especially in public methods. Throw an exception if the method cannot correctly do what it is supposed to do with the passed in argument(s) and/or the passed in arguments almost certainly indicate a bug in the calling code.
[*] Don't treat exceptions as scary, nasty things that crash your program. Instead, think of them as helpful tools that help you quickly identify unexpected problems and bugs in your program.
[*] Don't be irrationally afraid of letting the program crash. If there is a problem in your code that you cannot deal with, let it crash immediately so that you can be promptly made aware of the problem. This is often much more preferable than ignoring the problem and trying to continue, and thinking your program works brilliantly just because it isn't flat out crashing!
Imagine that you then give a demo to your customer having ignored a problem without really fixing it, and your program is suddenly producing incorrect results ultimately because of that problem you sat on weeks ago, but you have no clue why it is happening by this point, and you have no explanation for the customer. No thanks. I'll take the horrible red flashing lights crash during development instead .
2) As andrewsw mentioned, you should look into using the using statement around your StreamReader. This will ensure that the reader is closed even if ReadLine() (for example) produces an error (specifically, if it throws an exception). In your code, if any of the code in between the definition of myReader and the line myReader.Close() throws an exception, myReader will not be closed, and the file will remain marked as 'In Use', even though it no longer is.
Point to take away
Always clean up any resources that you use as promptly as possible, and try to make any cleanup as resilient as possible in the face of exceptions. Liberal use of the using statement will help you achieve this in C#.
Other points to consider
I think you've done quite well with the naming of your variables and method in general. Much better than many newcomers.
A few things to consider though...
Standard .NET naming conventions say that method names should start with an upper case letter. This may seem like pedantry, but solid adherence to conventions (whether it be Microsoft's, or just internal conventions used within a company or a project) is important because it helps give your code a critically important quality - consistency. As a developer, you should be able to adapt to your surroundings, and write code in a manner that is consistent with the code base you are working on at the time. Most C# code you see will adhere to this particular convention, and so you should to in your own code.
Also, some of your names focus a little to much on the mechanism you are using to achieve your goal. I would prefer to see names that describe in more real world terms what the variables/methods are representing/doing, rather than how they are representing/doing it. Doing so removes clutter, and makes the process of reading the code flow that little bit better. E.g. lineArray should become lines, and setTxtDataToArray should become SetTxtData.
Also, rather than linesOfTextInDoc, I'd probably name that parameter linesToRead. I think that more clearly demonstrates what that parameter represents.
In addition, be careful in your use of abbreviations (like txt) in method names (especially public ones). Such abbreviations are more accepted in variable names, but they should always be critically evaluated before use to ensure readability isn't compromised.
Finally, drop any extraneous words that add nothing to the clarity of the name. For example, myReader could be just reader.
Point to take away - The readability of your code is of paramount importance, and you should start actively thinking about the readability of your code now, in order to build good habits. Well named methods and variables are an important step towards making your code readable.
Passing in key dependencies
Again, andrewsw touched on this. A dependency is some service or piece of data that your method (or class) requires in order to do its job. In your method, the key dependencies are the number of lines to read, and where to read them from.
Therefore, you should pass the file path as a argument to the method, rather than hard coding it within the method. This allows you to use the method with any text file you want without having to constantly change your method. Suddenly, your method that was only useful for reading a text file at a particular path with a particular name, is now useful for reading any text file at any path! That is a pretty big win, and has increased the flexibility of the method considerably.
(You could take this even further if you wanted/needed to by passing in a TextReader instead, but that's one for another day )
Point to take away - Design your methods and classes so that they ask for the key things that they need as parameters so that you can configure their behavior without changing their code.
Return the array that you build
Your method currently just builds the array, but doesn't return it, so code that uses the method can't actually use the array you build. Therefore, you should return the array from the method to make it available for use.
Use the highest level tool that will do the job
This basically amounts to being as lazy (and smart ) as possible. You want to use the tool that minimizes the amount of work you have to do, and maximizes the clarity, simplicity and expressiveness of your code.
In this case, that could amount to using a List<T> instead of an array (a list is really just an array with a few extra features to make life easier).
Using a list would mean every item in the final collection of lines would be non-null (because when you loop over a list, it only returns items you have actually added to it, and you only ever add non-null lines), meaning you could remove the null check in your foreach debug code.
By using an array, if there are less lines actually in the file than there are lines you want to read, you get null items at the end of the array. A collection of items where only some of the items are valid isn't very nice to work with.
Similarly, you could use a for loop instead of a while loop, which may arguably communicate your logic and intent in a clearer manner in this case, since a for loop is specifically designed to iterate a set number of times. This is debatable though.
Point to take away - Try to use the tool that demonstrates your intent most clearly (this is often the tool that most closely resembles the real world concept you are modeling), makes your life (and everyone else's) as easy as it can be, and results in the simplest code to read and use.
Example Applying These Suggestions
This code still suffers from one problem though. It allocates memory for a list of linesToRead lines when there may in reality be far fewer lines in the file, hence it potentially wastes memory. One solution to this is to make use of the yield keyword.
However, the best solution is to make use of the highest level tools we have available that will do this job, and simply do:
- Member Title:
- D.I.C Addict
- 23 years old
- June 11, 1992
- United Kingdom
Music - Led Zeppelin, Guns N' Roses, AC/DC
Software Development and Design
Watching Supernatural, The Big Bang Theory, The Walking Dead, Red Dwarf, QI, The Simpsons, Breaking Bad
Online gaming (Xbox One).
- Full Name:
- Years Programming:
- Programming Languages:
- C#, a bit of Actionscript 3.0 ,SQL,Progress4GL, VB.NET. Learning Java, and Assembly/C/C++ when I can.
- Click here to e-mail me
Showing 50 random friends of 34