136

Just a quick note: a Status (or ConnectionStatus) enum with ONLINE and OFFLINE values would be more readable here.


110

I think that having both properties is actually somewhat harmful. If there is just single property, then it's clear what it means: settings can be either online or not. But when you have two properties (and don't know their implementation), then you start to wonder: is there some third case? Can both properties be true, or both false? Are the two properties ...


62

I felt compelled to write that you probably should combine forEach and map with the answer of Alexey Lebedev. function groupBy( array , f ) { var groups = {}; array.forEach( function( o ) { var group = JSON.stringify( f(o) ); groups[group] = groups[group] || []; groups[group].push( o ); }); return Object.keys(groups).map( function( ...


24

The main problem with your function is quadratic time complexity in the worst case. Also, if we first implement a general groupBy function, grouping by properties becomes trivial. function arrayFromObject(obj) { var arr = []; for (var i in obj) { arr.push(obj[i]); } return arr; } function groupBy(list, fn) { var groups = {}; ...


17

If you want to keep the two lists (otherwise check Hackworth's answer), the best solution would be to use LINQ extensions public IEnumerable<Page> AllPages { get { return HeaderPages.Concat(SurveyPages); } } This way you do not create any unnecessary collections. It forces you to use IEnumerable<Page> though, but in my ...


17

If you implement both fields you pretty much have to guarantee for the rest of your life that they will always be consistent, i.e. that IsOnline==!IsOffline is always valid, at any point in time, forever. Can you? What if someone extends your class and decides to "extend" the logic, too. public class MySettings : Settings { public bool IsOnline { get; ...


15

Methods Set and Get Properties through their Get and Set methods, so it isn't a really good idea to perform operations on properties inside their get and set methods. you should use the object methods to do this. public string DisplayErrors { get { var errorString = ""; if (HasErrors) { errorString += Issues....


15

I think that most things are said, I recapitulate and add some of my own stuff: As Jesse said, I'd use Any instead of Where + Count. It should have a better performance, I think. Properties are for me: HasNoIssues, HasWarnings and HasErrors In my Opinion the rest should be methods Issues.Where(x => x.Name == "Warnings").First() is the same as Issues....


13

I would do this using an interface and polymorphism to represent this: public interface ISession { void DoSomething(); // Add your properties / method definitions } public class ServerSession : ISession { private readonly string _serverName; public ServerSession(string serverName) { _serverName = serverName; } ...


13

If you have control over the design of the Page class, I would add a "bool IsHeader" property to it and keep one public list of all Page objects and add every Page object to that list. Instead of keeping several other lists that sort your Page objects, you can then perform searches on that master list instead for Page objects with certain properties, ...


12

I would avoid the additional property. It's entirely unnecessary. The benefits of including it would be to improve readability (ever so slightly) as suggested by ChrisWue's answer. Other than that it poses no advantages that would outweigh the confusion it could cause as suggested by svick (a real consideration to make if it's going to be a headache for ...


11

If you expose it via a public property then it's not a "private" array any more:it's part of the public interface. If resetting its elements is something that you want clients to do, I'd choose the first option (because it's simplest). Or, these ... public void SetColor(int id, Color value) { ... } public bool GetColor(int id, out Color result) { ... } .....


11

I don't do much asp.net, but if this StackOverflow answer applies then you could just do this: using Settings = MyProject.Namespace.Properties.Settings; var loadedCode = Settings.Default.LoadedCode; Like this other StackOverflow answer is saying: We prefer to use Properties.Settings (aka. settings.settings) because it's strongly typed. Now I know for ...


10

Basically after reading over the code, I interpret that the conditional (i.e. !obj[key].push) checks if the value at the given key is not an array. A more robust way to do that is to use (the negated value of) Array.isArray() instead. That may not be any shorter, but perhaps a better way to determine if the property at key is an array. var obj={a:1,b:[1,...


9

I find the functional aspect of JavaScript to be a big advantage. When it comes to looping, Array.prototype.forEach and cousins can help your code be more descriptive: Array.prototype.defineProperty('groupByProperties', { value : function(properties){ // will contain grouped items var result = []; // ...


9

No one has yet addressed the fact that you have chosen the wrong tool for the job. What does Config represent? It looks like you should have chosen an enum where you chose an int. public enum ConfigurationType { None = 0, Type1 = 1, Type2 = 2, // etc. } You can get what ConfigurationType an int corresponds to quite easily: ...


8

There are a couple of inefficient things you do, and there's some better-practice items too. First, let's take one of the methods: public static List<String> loadNodes2_00() { List<String> node = new ArrayList<>(); try { InputStream inputStream = XmlUtil.class.getClassLoader().getResourceAsStream("nodes2.00.properties");...


8

I tend to use monads in such situations: public static TResult With<TSource, TResult>(this TSource source, Func<TSource, TResult> action) where TSource : class { if (source == null) return default(TResult); return action(source); } public static TSource Do<TSource>(this TSource source, Action<TSource> action) where ...


7

Before worrying about delegates, there are some very basic notions we should review! :) Never, ever getting back together expose your fields. They should be private. Use properties like you did in your Quarry class instead. This pattern is called encapsulation and is an important practice in the software development process. In general, an instance (like ...


6

If "offline" is really just a "not online" in terms of business logic here - most probably is - then the method is superfluous. However, it is not uncommon to create similarly looking methods that from the looks of do not do anything at first glance, but serve as a placeholder to better indicate a test - and be a single point of extension should an ...


6

I would like to find a way to integrate declarations "property, get_property and set_property" into one. How can I determine object type from its constructor? Don't. Seriously. No no no. Everything about this is bad code design from the bottom up and you can object to it for every reason. Even allowing for the idea that you should use getters and setters (...


6

public bool TryGetProperty<TInstance, TFinal>(TInstance instance, Expression<Func<TInstance, TFinal>> propertyAccessor, out TFinal output) { var currentExpression = propertyAccessor.Body as MemberExpression; var stack = new Stack<PropertyInfo>(); do { stack.Push(currentExpression.Member as PropertyInfo); } ...


6

One thing I don't see you address, is that INotifyPropertyChanged.PropertyChanged has the signature of: public delegate void PropertyChangedEventHandler(object sender, PropertyChangedEventArgs e); where object sender should be the owner of the property (typically this). You don't provide the code for ObservableObject, but from your code above it seems that ...


6

I don't know which version of C# you use, but as of C# 6 it's possible to set initial values on property definition, which has 2 advantages: Can eyeball the initial values quickly by looking at the property definitions. Doesn't require you to copy the same initial value assignment code into additional constructors you may create. Example initial value ...


5

As you said, keep another List which will contains all the elements and when you add an element inside one of the two update the internal List ... you could make List private and make helper method which update the main list and the internal List. With your code anyway you will create a List every item it's called and I think it could lead to bugs if you ...


5

There is a different way, supplemented by documentation. One can say that it goes against OOP, one can also say that it keeps things simple and favors composition over inheritance. Here serverName = null is a convention for local connection. This approach condenses the entire state to a single string value. This is not the only way. Sometimes this is a bad ...


5

Yes, it can be beneficial to add a property like this: Testing for a positive outcome in a condition usually makes it a little bit easier to read especially if the condition is a bit more complex. E.g. if (!IsOnline || ForceReset) { TryReconnect(); } vs if (IsOffline || ForceReset) { TryReconnect(); } Highly depends on who reads the code though ...


5

As @palacsint mentioned I would create a Status property, but also I would create read-only boolean getters: public interface ISettings { Status Status { get; set; } bool IsOnline { get; } bool IsOffline { get; } } public class Settings : ISettings { public Status Status { get; set; } public bool IsOnline { get { return Status == Status....


5

First note that your var connectionState is computed property, not a stored property. It is computed using the get and set methods. Extensions cannot add stored properties to a type. Your current implementation crashes if the connection state is retrieved without having been set before: let provider = ServiceProvider() print(provider.connectionState) ...


4

I agree with the other answers to this question, particularly palacsint's, but feel they do not focus on what I consider the most important point: you are talking about an interface. For an interface, it is crucial that what you define be clear and unambiguous--if you provide two properties, then a consumer of the INTERFACE can not be sure that they are ...


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