Hot answers tagged

86

The rationale behind splitting functions is not how many times they will be called, it's keeping them small and preventing them from doing several different things. Bob Martin's book Clean Code gives good guidelines on when to split a function: Functions should be small; how small? See the bullet bellow. Functions should do only one thing. So if the ...


40

I think function naming is very important here. A heavily dissected function can be very self-documenting. If each logical process within a function is split out into its own function, with minimal internal logic, then the behavior of each statement can be reasoned out by the names of the functions and the parameters they take. Of course, there is a ...


27

I'd say that in the C# world, an enum would be one of the best options here. With that you'd be forced to spell out what you're doing and any user would see whats happening. It's also safe for future extensions; public enum WhatToInclude { IncludeAll, ExcludeManagement } var ids = ReturnEmployeeIds(WhatToInclude.ExcludeManagement); So, in my ...


26

Having a break out of a loop is no different than having that loop get refactored out to a function of its own and a return statement in a guard clause. while(condition) { if(test) { break; } doStuff; } vs doMuchStuff(); function doMuchStuff() { while(condition) { if(test) { return; } doStuff; } } Those are effectively the same. ...


18

does it make sense to write: var ids=ReturnEmployeeIds(includeManagement: true); It is debatable if this is "good style", but IMHO this is as least not bad style, and useful, as long as the code line does not become "too long". Without named parameters, it is also a common style to introduce an explaining variable: bool includeManagement=true; var ...


16

If you encounter code like: public List<Guid> ReturnEmployeeIds(bool includeManagement = false) { } you can pretty much guarantee what's inside the {} will be something like: public List<Guid> ReturnEmployeeIds(bool includeManagement = false) { if (includeManagement) return something else return something else } In ...


12

Any time you feel the need to write a comment to describe what a block of text is doing, you have found an opportunity to extract a method. Rather than //find eligible contestants var eligible = contestants.Where(c=>c.Age >= 18) eligible = eligible.Where(c=>c.Country == US) try var eligible = FindEligible(contestants)


8

Use the package name. This type of problem is precisely why Java uses the package naming convention that it does. It prevents these sorts of problems, whether it's two teams in the same company or two teams on opposite sides of the earth.


6

DRY - Don't repeat yourself - is just one of several principles that have to be balanced. Some other that come to mind here are naming. If the logic is convoluted not obvious to the casual reader, extraction into method/function whose name better encapsulated what and why it is doing it it can improve program readability. Also aiming for less than 5-10 ...


5

What about creating a class to hold your arguments? This class would contain both open and close parameters and either of them could be NULL. Then, there will be only one strip method with above class as argument and method will decide if it wants to use open/close if they are set.


4

The point about splitting functions is all about one thing: simplicity. A reader of code cannot have more than about seven things in mind simultaneously. Your functions should reflect that. If you build too long functions, they will be unreadable because you have much more than seven things inside your functions. If you build a ton of one-line functions, ...


4

There are many options, it's your tradeoff which to take: Decision at runtime: Add a defaulted bool argument: QString MyClass::strip(QRegularExpression regex, bool close=false); // Mimic the two-regex-variants interface as good as possible Use scoped enum's and no default as a variant on 1 which is more descriptive: enum class option { open, close }; ...


3

As I commented, yes you should usually check against nullptr and always document if a public function accepts a null pointer or not (that it, document a function as accepting a null pointer, or as requiring a non-null valid pointer). Checking against a nullptr is -on most current C++11 implementations- very quick: dereferencing a pointer might make a cache ...


3

I would suggest QString MyClass::strip(); QString MyClass::strip(QRegularExpression regex, bool opening=false); QString MyClass::strip(QRegularExpression open, QRegularExpression close); and perhaps QString MyClass::strip_open(QRegularExpression regex); QString MyClass::strip_close(QRegularExpression regex); or replace the bool opening with e.g. enum ...


2

Kind of echoing Basile here but with a slightly more negative tone. I'd say "yes" too, but be careful with that if you are allowing silent violations of preconditions. As a personal example, I worked in a C codebase that did this kind of stuff through an API: int f(struct Foo* foo) { if (!foo) return error; ... return success; } ...


2

@MichaelT asked how to rewrite the C# example code for goto without using goto. Here is their code: using System; class Test { static void Main(string[] args) { string[,] table = { {"Red", "Blue", "Green"}, {"Monday", "Wednesday", "Friday"} }; foreach (string str in args) { int row, colm; for (row = ...


2

The main concern I have found with logging close to the source is that you are in danger of logging the same errors twice. In the second case we might imagine someone calling DoSomethingElse() without being sure whether it would log the error. These cases become increasingly common as the size and complexity of the component increases, and the number of ...


1

Main adventage of second solution ( pass errors up ) is: when you run mutiple functions in row that can throw errors, you don't have to make if for each one, but you can just do one if on higher level. And this solution is also encouraged on Go lang blog ( Simplifying repetitive error handling section ).


1

Quoting https://www.python.org/dev/peps/pep-0008/#package-and-module-names: Modules should have short, all-lowercase names. Underscores can be used in the module name if it improves readability. Python packages should also have short, all-lowercase names, although the use of underscores is discouraged. And for classes: Class names should normally ...


1

It's all about separation of concerns. (ok, not all about it; this is a simplification). This is fine: function initializeUser(name, job, bye) { this.username = name; this.occupation = job; this.farewell = bye; this.gender = Gender.unspecified; this.species = Species.getSpeciesFromJob(this.occupation); ... etc in the same vein. } ...


1

The "right" answer, according to the prevalent coding dogmas, is to split large functions into small, easy to read, testable and tested functions with self documenting names. That said, defining "large" in terms of "lines of code" can seem arbitrary, dogmatic, and tedious, which can cause unnecessary disagreements, scruples, and tension. But, fear not! ...


1

Pretty much depends on what your // Logic Here is. If it's a one-liner, then probably you don't need a functional decomposition. If, on the other hand, it's lines and lines of code, then it is much better to put it into a separate function and name it appropriately (f1,f2,f3 does not pass muster here). This is all have to do with human brains on average ...


1

As of now you have one ModelDevice class (Device in the model package). What if you have another such ModelDevice for a different classification? The problem may still persist and the overheads will also continue to increase. Though for the time being you may find that renaming classes be of some good help, for a long run the suggested alternate is to go ...


1

Where this style starts to be a little awkward is when you introduce public methods: function ParentFunction() { var myVariable; var scope = this; // Keep scope stable inside enclosed methods. function parentFunction() { sonFunction(); // no 'this'/'scope' this.daughterFunction(); // needs 'this'/'scope' } function sonFunction() { ...


1

Doesn't anything that mutates eventually manipulate state? Yes, but if it's behind a member function of a small class that is the sole entity in the entire system that can manipulate its private state, then that state has a very narrow scope. What does you should have to deal with as little state as possible mean? From the variable's standpoint: ...



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