Comments (3)
It's kinda hard to discuss the merits of making this more object-oriented without actual code, but at this point I'm very skeptical that this will be an improvement. Addressing your motivations for bringing this up:
Since there is no internal state (= no members), the options variable is passed around all the time.
Actually, only _preprocessToStdout
and _outputFileFromArgument
use the options
argument. _preprocessToStdout
is an unused method and _outputFileFromArgument
could be refactored to not need options
without too much trouble. Actually, there is only one caller of _outputFileFromArgument
, so it might be viable to just inline the call.
The name "analyze" is not helping a lot.
I agree, but having the term 'analyze' in a call FooAnalyzer.analyze(cmdLine)
seems as useful (i.e. not very much) to me as writing CommandLineAnalysis(cmdLine)
. Making this object-oriented doesn't magically make it more expressive. I believe an actual improvement would be to be more explicit about what the function does: it deduces the source and output files given some command line.
The output if analyze() is an unnamed tupel[sic!]
In this particular case, I see no issue with having a function return a tuple. It actually permits assigning names to the tuple values on the callsite, i.e. given def f(): return 47, 11
you could write foo, bar = f()
. If instead you return an object, you write o = f()
and then have to use o.foo
and o.bar
-- an unnecessary indirection.
We cannot test the content of options
We can, this would just be a matter of calling CommandLineAnalyzer._parseOptionsAndFiles
. It would be fine to make this public (i.e. by removing the leading underscore) for this purpose. In general, having free functions whose behaviour is only defined by their arguments (i.e. they are "pure" and free of side effects) is ideal for testing. No setup or teardown work is required, no objects to be greated. Just call a function with arguments and check the output.
static functions cause long caller code (e.g. CommandLineAnalyzer._outputFileFromArgument)
You don't need to reference the class object when calling static methods (this is only true for Ruby, not for most other languages like C++ or Python). The code could just as well use self._outputFileFromArgument
. It is however not uncommon to see fully-qualified calls to express that the called method does not depend on the member variables but only on globals and arguments.
IIRC, the code currently only uses a class CommandLineAnalyzer
to implement local functions. A simpler way to achieve this might be to just introduce a new file (say, cmdline.py
) which exposes individual functions (preferably with short names) related to handling the command line, e.g.:
cmdline.expand
(corresponds to currentexpandCommandLine
function) which recursively expands command files. It takes a string list corresponding to the argument vector and returns a new string vector.expand
is hence idempotent.cmdline.parse
(corresponds to currentCommandLineAnalyzer._parseOptionsAndFiles
) takes care of parse an argument vector (i.e. a string list) into a dictionary of options and a list of source files.cmdline.getInputOutputFiles
(corresponds to currentCommandLineAnalyzer.analyze
) is the main entry point. It takes an argument vector, expands it, parses it and then determines the input and output files involved in the invocation.
from clcache.
Okay, fine. There is no real need to change it, I just thought it makes a bunch of places slightly better. But I am perfectly fine keeping it as it is :)
Let me just answer a few things:
- I did not see that
_preprocessToStdout
is unused, PR is out. - Inlining
_outputFileFromArgument
is good. It makes the code more concrete. - Making
_parseOptionsAndFiles
public is okay. I would avoid testing private interfaces, which is considered bad design. - I had the impression that you dislike the word "analyze()" more than me ;)
- I don't think it's a good style to call static functions in a member fashion. But the longer names don't hurt, right.
- I would like to avoid splitting the project in multiple files as long as possible. It is very handy with just over 1000 lines to have everything in place. More files mean more license headers and linting. Sure, it can be done if really useful, but I don't see that.
from clcache.
I guess there's no action to be done here, so let's close this issue.
from clcache.
Related Issues (20)
- Error installing latest on Windows 10 HOT 5
- Renaming build directory cause cache misses HOT 2
- Clcache performance issues with big projects HOT 5
- What does "passed for external debug" mean? HOT 1
- Syntax error (printTraceStatement) running on Windows 10 HOT 2
- Please state that python 2 is no-longer supported in the readme.
- Autoclean feature documentation HOT 2
- Error "Cannot find file at '..\\lib\clcache\clcache\clcache.exe'" HOT 1
- ClCache mess up with MSBuild cache because of STATS.TXT HOT 2
- Can't use clcache from terminal when using VS HOT 3
- Visual Studio IDE w/ CMake w/ clcache always rebuilds HOT 5
- Enabling clcache overrides the local property-sheets
- Migrate LGTM.com installation from OAuth to GitHub App
- Enable long paths support on Windows 10
- Warning ModuleNotFoundError
- Update Visual Studio Integration Docs
- CLCACHE_PROFILE ignored when clcache is installed as .exe from chocolatey or pip HOT 1
- ThreadPoolExecutor / concurrent.futures.as_completed prevent cProfile from providing actual info
- Alternative CMake Integration HOT 4
- Problems with ASM identification, using clcache on CMAKE
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from clcache.