30
Type type = input.GetType();
MemberInfo[] memInfo = type.GetMember(input.ToString());
You should use more meaningful variable names like enumType and enumMembers and your code could use some more LINQ and vars.
if (memInfo != null && memInfo.Length > 0)
This null check is unnecessary. The docs says about GetMember:
An array of MemberInfo ...
20
First of all I'd reduce indentation. It makes your code really too hard to read. Doing that you will see it may be simplified.
First of all try/catch: a catch-all block where you rethrow the exception is useless (and you may even throw away line number information.)
This code:
if (obj != null)
{
// Do something...
}
May be replaced simply with:
if (...
13
Naming:
Don't use variable names such as t. They're not meaningful, not for you, not for others. In this case, use type.
Class names, public fields and method names use PascalCase and not camelCase. So, isEmpty will become IsEmpty. But then again, this is also not a meaningful name. Boolean methods or properties in C# will most likely start with Is or Has. ...
13
Small things
Your code styling is generally good, but a few possible improvements:
You can add a second generic type parameter to your method to avoid the need to cast to object. i.e. public static List<TIn,TOut> MergeListData<TOut>(List<TIn> collection)
You can probably use a more general collection than List<object> for your input ...
11
Python and the power of unpacking may help you in this one,
As it is unclear how your Class is used, I will give an example of how to initialize the dictionary with unpacking. This will work on any iterable.
True to it's name, what this does is pack all the arguments that this method call receives into one single variable, a tuple called *args. The same ...
10
You're loading every DLL in the directory even if you find what you want in the first one.
I'd guess that loading an assembly is what takes more time and resources than looking for what you want in the assembly-after-it's-loaded.
You code will fail if there's a type which implements IDataAccess but which doesn't have an Instance property.
You might want ...
10
A few points tickle:
Why is it a static method?
None of the comments are helpful. Good comments say why, not what. I'd remove them all.
Bracing is inconsistent. Consider:
if (!(dr[propertyInfo.Name] is DBNull))
propertyInfo.SetValue(ob, dr[propertyInfo.Name]);
And then:
if (!(dr[primaryKey] is DBNull))
{
propertyInfo.SetValue(ob, dr[primaryKey]....
10
Method header
Your method header can be improved, the generic type E is not necessary, it can instead be written as:
public static <CustomEnum extends Enum & HumanReadableEnum>
List<String> toHumanReadableCollection(Class<CustomEnum> enumClass){
Type parameters should generally be one-character names, so this would be better as:
...
10
Personally I wouldn't do this.
The reason why is when your field change name, your test fails because the field is hardcoded there and with refactoring this isn't persisted to the test.
What should I do?
Normally your AuthorizationBeanTest is in the same package as your AuthorizationBean.
When this is correct you could use a protected method.
Example :
...
10
readonly will not make members of your static serializers readonly. While you cannot reassign another serializer to replace it, its members can still be modified. Since you have access to C# you can use a get-only property to return a new instance :
public static DelimitedSerializer TsvSerializer => new DelimitedSerializer { ColumnDelimiter = "\t", ...
10
for (int i = 0; i < propInfos.Length; i++)
Any particular reason why you're not using foreach (var prop in PropInfos) here?
catch (Exception ex) { if (!string.IsNullOrEmpty(ex.Message)) return default(List<T>); }
I consider this line to be dangerous. By using Pokemon exception handling ("gotta catch 'em all") and silently swallowing it, you ...
9
Just a few things to point out:
There's no need to use inline yourself. For modern compilers, it merely serves as a suggestion, but they can otherwise determine if and when it's really needed. Read this for more info.
You should use consistent naming for your namespaces (one is lowercase and the other is uppercase). I'd not use uppercase as it's commonly ...
8
I took inspiration from @tim_yates answer and defined a Mappable trait which handles nested Mappable objects. The Trait (as opposed to a class) will make your asMap method more generically applicable, which is one of the goals you have stated.
trait Mappable {
Map asMap() {
this.metaClass.properties.findAll{ 'class' != it.name }.collectEntries {...
8
Any exceptions at this point will be fatal, and are trapped and logged in the calling code before a (hopefully) graceful exit.
I think you have a use case for a custom exception type here. Throwing System.ApplicationException isn't much better than throwing System.Exception directly. It's actually a relic from the early days of .net; System....
8
It ignores values in superclasses. You might want to check that too.
Anyway, don't reinvent the weel, there is a library for that! I guess EqualsBuilder.reflectionEquals from Apache Commons Lang does exactly what you want. It also has solutions to corner cases, like
transient fields (since they are likely derived fields, and not part of the value of the ...
8
LINQ has two syntaxes to offer. You've used the method syntax, which I find very neat and useful, but for simpler queries I find the query syntax looks much less bulky in VB.NET:
Dim result = (From p In GetType(TObject).GetProperties()
Where p.Name = PropertyName _
AndAlso p.CanRead() _
AndAlso p....
8
As written in the relative MSDN page, the cause of the warning is
The parameter signature of an externally visible generic method does not contain types that correspond to all the type parameters of the method.
Looking at your code, you get the type of the generic param into the method body (which can be seen also as a violation of the Separation of ...
8
private const string Space = " ";
There's little point in storing this in a variable: it's never going to change anyway and you're not clarifying any of the intent. I'm fully aware that is a space and { is a left curly bracket.
Using dynamic is a very, very expensive action. By defaulting to it everywhere instead of using an intermediate type you're ...
8
I wouldn't go the StringBuilder route, simply because you need to handle the "what if I'm at the start/end" problem. Instead, simply collect the "URL_translated" key-value pairs into an IEnumerable<string> and then apply string.join().
If you're working in .NET Core, Microsoft does all the lifting for you. Here's how they do it.
8
In places where the type is obvious (or irrelevant) then feel free to use var instead of the type name. Also, consider not naming your variable after the type; this is also generally not necessary.
I also don't think that you need to make props a list.
A trailing ampersand in the querystring is perfectly legal, so no need to special case it. You can handle ...
7
You should check if the references are equal first to save a whole lot of comparing when there is no need to.
If both objects are null your method will return false which is probably unexpected.
You call getClass() several times on these objects - It should only be called once for each object and then stored in a local variable.
There is a typo in ...
7
T ToEntity();
To me, ToEntity() implies some sort of conversion action. A better option might be T GetEntity() or even a property called Entity.
protected readonly T EntityType;
EntityType is a bad name for this field, because it does not contain a type, it contains the entity. Because of that, something like Entity might be better.
Also, you might want ...
7
According to your comment,
if it's only the MultipartFileWrapper.class.getField("multipartFile") statement that might throw a NoSuchFieldException,
then when that happens, the lines that set imageFormats and videoFormats will not be reached, so these values will remain null, and the if conditions in the catch block are pointless.
Furthermore, it's good to ...
7
The answer from @chillworld answer your question but there is still some really smalls details in your code.
I like that you're using import static for the asserts statement. This leave the code free from the repetitive Assert.assertNull.
The problem is you're cluterring the code with comments that serve no purpose.
//Mocking the static function ...
7
Naming
Type enumType = typeof(T);
this name is misleading because one would assume that its always an enum which you later check by reading the IsEnum property. So just name it type or pay the price and call typeof(T) twice.
General
I usualy tend to return a empty List<T> instead of null because that makes any null check redundant.
I would suggest ...
7
For starters, I feel inclined to go meta and question the design. One possible shortcoming of your method is that it falls back to the default value, which - for primitives - isn't null. So "blah blah" would evaluate to (int) 0 or (bool) false, and unlike TryParse, your method won't tell me if this result actually came from the input, or whether it only ...
7
You use IList<> where you should use ICollection<>. I've rarely encountered a scenario where IList<> actually needs to be used. The ICollection<> interface has most of the list's methods, but without everything related to indexing, which you don't use anyway. It's not that big of a deal, but I think it's good knowledge.
When you ...
6
I would make some changes to getFirstUpper():
public String getFirstUpper(String str)
{
Integer x = str.length();
str = str.substring(0,1).toUpperCase().concat(str.substring(1, x));
return str;
}
The name x is not a very good name as it's only a single character. I'm not even sure if a separate variable is needed for this, but if so, consider ...
6
One objection I could raise is that your generic type constraint doesn't guarantee what you think it might guarantee. Eg. you could have:
// TSelf is actually not T of Self
public class HelloQuery : IQuery<GoodbyeQuery, string>
{
}
// This one is fine
public class GoodbyeQuery: IQuery<GoodbyeQuery, string>
{
}
In other words, TSelf doesn't ...
Only top voted, non community-wiki answers of a minimum length are eligible
