Hamming exercise submission to exercism/xcpp#1
Hamming exercise submission to exercism/xcpp#1sunzenshen wants to merge 11 commits intoexercism:masterfrom sunzenshen:master
Conversation
|
For some reason, I forgot to check the forks of xcpp beforehand. So I didn't notice that @DarthStrom already has examples ready, including the hamming exercise. Oh well, I learned some new things and hopefully this CppUTest implementation will be useful to compare with a Google Test one. |
|
No worries. I haven't touched it in a while - feel free to grab anything from it that you think is helpful. The only reason I chose google test is because that's what I'm currently using at work. |
|
I like that both brew and apt-get are available for CppUnit. Is that the case for Google test also? |
|
No, they recommend compiling yourself. https://code.google.com/p/googletest/wiki/FAQ#Why_is_it_not_recommended_to_install_a_pre-compiled_copy_of_Goog |
|
Ok, in that case I think I'd rather go with CppUnit (unless there are other compelling reasons to choose google test). @DarthStrom Would you mind giving this PR a review? I don't write C++, so I'm kind of useless here. |
|
Sure, no problem. |
|
Looks good to me. The one suggestion I would make is to maybe try to get rid of the complex directory structure and get the Hamming.cpp and HammingTest.cpp into the same directory. |
Consequence of moving Hamming.cpp and HammingTest.cpp to the same directory.
|
Really like the idea of simplifying the directory structure. The CMakelists organization is now less modular than when it was split into separate folders, but this is less of a concern given the small scope of these exercises. Hopefully having all the source files in one place is not too overwhelming? |
|
I like that a lot better. Can we also get rid of the .h and just include the .cpp directly? |
Idea is to have participants submit a single .cpp file to exercism.
|
I'm a little worried that including the .cpp file will set a bad example for inexperienced C++ learners, who don't know about the preprocessor. So I added a short warning in HammingTest.cpp. Hopefully this approach will be more of a learning opportunity than the corrupting of innocent minds. At least now participants only need to worry about a Hamming.cpp file, and not the header file. |
|
Yeah, I guess it's ultimately up to @kytrinyx whether the focus should be on the code itself or if project setup idioms should be a focus as well. Otherwise, I think this looks great! |
|
On second thought, you're right. It's a tradeoff between best practice and simplicity, but I think it's probably better to teach it right in this case. |
|
Okay, let's revert back to providing a header file. One idea that came to mind was maybe omitting the header file too. However, it seems like a common convention in other exercise repositories to have the participant create one file, and upload that as a submission. For example, the xscala exercises involve creating some folders in the "src" directory, but they involve creating only one exercise file. I'd prefer to provide a header file, so that it's more explicit what a participant needs to do. Defining the header files hopefully won't limit the creativity and expression of coding styles. Or to put it another way, there should be enough ways to shoot oneself in the foot with just a cpp file. |
|
I think we should prefer right over simple. The idioms and practices are different in each language, and part of being a good citizen in a language is knowing how to follow the principle of least surprise. |
|
In the spirit of teaching idioms, there would be more discussion points if the participant had to define the header file. One thing to point out with CppUTest is that the prototype is not limited in the tests. If we leave out the header file (i.e. renaming Hamming.h to example.h), then it would be up to the user to define what types they want to use. For example, my example implementation returns an int: int Compute(std::string, std::string);Another submission could have defined the return value as an unsigned int: unsigned int Compute(std::string, std::string);Both of those prototypes satisfy the tests, which don't specify the types of the input and output: The only sticking point is that as far as I know about exercism, you can submit only one file. The question of whether a header file needs to be submitted may become an FAQ in of itself. I'm probably over thinking this though. In the C++ exercise documentation, we could ask that only the cpp file be submitted, and the contents of the header file could be inferred. The blind spot with that approach is if the absent header files have mistakes in them, such as a lack of macro guard. |
|
Yeah, I've been meaning to make it possible to submit multiple files, but I haven't made the change yet. |
|
👍 |
|
With or without support for submitting multiple files, I don't think it will hurt to begin building the exercise count for this repository. It should be easy to "remove" header files from the exercise if that approach is chosen, and that decision can be postponed until there are 10+ exercises available. |
|
I've been doing the exercises for C++ here: https://github.com/LegalizeAdulthood/xcpp/tree/legalize I'm pretty far along and sent Katrina an email and she pointed me to this thread. I chose Boost.Test for the unit testing framework because it is widely available for different platforms, can be installed prebuilt easily on Unix and on Windows and you will need other libraries from Boost in order to do some of the exercises anyway. (Like gigasecond, which requires date arithmetic.) Libraries in Boost often migrate into the C++ standard, so encouraging people to get familiar with Boost while they do the C++ track is a good idea, IMO. I also used CMake for my build but I didn't include that in the github repo. I haven't yet reviewed all the comments on this thread, but I wanted to at least drop in and let people know what I was doing. I'll review this thread in its entirety this weekend. |
|
I'm happy to go with whatever unit testing framework. At first I kinda objected to the idea of bringing in a 3rd party library, but Boost is so commonly used it can't hurt to ask users to install it. As another data point, Homebrew for OSX also supports a "boost" recipe. Regarding CMake files, I think there should be a discussion about whether or not users should also worry about getting the unit tests to compile. Is there a possibility of exercises becoming more about makefile organization than the actual exercises? Sites like Hacker Rank handle the compilation of C++ code, and it may be helpful to provide similar ease of use. On the other hand, making it the participant's responsibility to get compilation working would remove the maintenance effort that comes with providing makefiles. At the very least, there should be some instructions on how to compile with (insert unit test framework here), if related files are not included. |
|
This thread looks like it could be going towards a similar discussion regarding user accessibility: If users could get confused figuring out how to run a simple Go program, getting C++ to compile is likely to be an even bigger project in itself. |
|
Part of the difficulty here is that some users are completely new to programming, and all the various pieces of setup can seem very arbitrary to them. With people familiar with programming, understanding all the necessary pieces of setup/compilation/whatever is just another part of doing it right. I don't know what the right answer is. I suspect that the most important thing is to improve all the tools, explanations, READMEs, etc so that it's easier to get an early success with the first problem. |
|
Is it possible to have files pushed to the language specific directory when they run the exercism command to start the first exercise for a language track? If so, then I could push a CMakeLists.txt to that directory that solves most of their build problems. Take a look at the instructions I put in my SETUP.md: https://github.com/LegalizeAdulthood/xcpp/blob/legalize/SETUP.md |
|
I recently did a workshop on TDD using Boost.Test. In my dry run for the workshop, I had people configure their own build environment before starting. This took approximately 30 minutes just to get a simple static library linked against the console test executable. It was a giant distraction. For the actual deployment of the workshop, I gave them a CMake recipe similar to the one I described in the SETUP.md file. This got them going in about 5-10 minutes after they had the necessary stuff installed (CMake + Boost). CMake gives you IDE workspsaces for Xcode and Visual Studio, as well as generating Makefiles if you're using Unix. So I really think CMake is the best solution to "getting all that build stuff out of the way" and focusing on writing some C++. |
|
We can make it possible to send along a build file, it should be pretty straight-forward. We could add a section to the config.json that tells the API what to send along (filename) and then just deliver it with every exercise and only write it if it is missing. Or something :) |
|
I like the idea of writing the file if it is missing. I need someone to beta test my proposed recipe :). My boost setup is a little odd because I've got dev trees and stuff to support me contributing to boost. |
|
I'm afraid I'm not the right person to beta test the recipe unless you want a complete C++ n00b ❤️ |
|
Actually, that's perfect! If a n00b can do it, then we did it right :) |
|
Agreed, complete C++ n00bs would be the perfect individuals for test driving any build recipes. Since we already have example solutions available, they don't even need to know how to write C++. ;) I would be happy to beta test the recipe, with the caveat that I already have most things installed from exploring C++ unit test frameworks. In absence of exercism support (eg: the language specific directory), maybe there could be a small temporary repo, with a proposed layout of files for a first time user? An example exercism/cpp directory would also clarify how to implement a new config.json entry. |
|
FYI, as per an email discussion with Katrina, I will be pushing to the master branch of my clone of the repo. I'm discontinuing use of the 'legalize' branch I had created. If a pull request is honored for xcpp, does that mean the exercises will show up to anyone using exercism currently? |
|
That works. I'm totally fine with closing this pull request, as the legalize branch looks great. Excited that the xcpp track is making forward progress. 😄 |
I actually prefer that you use branches, not master, for pull requests: One branch per exercise. If you found the response confusing, please email me so we can clarify. |
|
Gack, I misread what you wanted and inverted the sense. This is the first time I've issued a pull request on github, so I'm a github n00b! |
|
No worries, I'm not always very good at explaining things, so this one is on me. |
|
I think you did fine, I just confused myself being a github n00b :). OK, I updated description in SETUP.md to say how to get cmake to generate your project setup (i.e. run |
|
OK, cool. I'm closing this PR. @sunzenshen and @DarthStrom - I'll ping both of you to help review the PRs when @LegalizeAdulthood submits them, if that's OK. |
This first exercise submission will hopefully spur some discussion regarding how the xCpp track should be implemented. The hamming exercise folder seems a bit heavy as is, so there's no doubt that it could be streamlined. And the choice of unit test framework is likely up for debate.
I chose CppUTest as the unit testing framework, partly because it had a large number of stars on GitHub. Support for apt-get and brew didn't hurt either.
This is how I set up my environment on OSX:
(Where RunAllTests is located in xcpp/hamming/build/bin/)
Sources of research: