Comments (16)
@Mahir1015 Thanks very much for looking into this! However, I am not a part of The Odin Project and hence am not able to assign issues or merge PRs, you may wish to approach someone on the discord who has the access to do so
from javascript-exercises.
@Mahir1015 You can find it on this page: https://www.theodinproject.com/lessons/foundations-join-the-odin-community
from javascript-exercises.
Thanks @thatblindgeye for the offer, I'm not interested in performing the update
from javascript-exercises.
Can you please please assign it to me? I've already passed all the tests. As soon as you assign it to me I'll make a pull request.
from javascript-exercises.
@Houndoom I've made a PR on the following issue. Please review it.
from javascript-exercises.
@Houndoom Anytimee! Can you please provide me the link of discord server?
from javascript-exercises.
@Houndoom Thanks!
from javascript-exercises.
@Houndoom would you be able to explain further the issue with the 4th test in this exercise? If a test fails, the intent is that a user refactors their function(s) until that test (and all previous tests) pass. In this case the test should pass by returning a string "ERROR" if any negative numbers are passed to the function.
from javascript-exercises.
@thatblindgeye There is nothing in the question that suggests that the task should fail for negative numbers. The term "integers" includes negative integers as well, and hence I would expect this function to work with negative integers. If the intention was for it to fail with negative integers, suggest to amend the question to state "positive integers".
from javascript-exercises.
@Houndoom Ah okay, I see what you mean. You're correct, we should either update the README to mention the function should only work for positive numbers, or add/update tests to work with negative numbers. Do you feel strongly one way or the other, coming from the point of view as a user?
from javascript-exercises.
@thatblindgeye I have no strong preference, but perhaps a slight preference for updating the README as it would be a good introduction to edge cases, and more learning is always good. If this route is taken, suggest to have an appropriate hint to guide the user to think about the edge case, and the desired return value of "ERROR" would have to be explicitly stated.
from javascript-exercises.
Sounds good to me! I'll open this for help wanted and update your original comment with what needs to be done
Actually as I type out a task list and think it over a little more, I'm thinking we may not really need to really mention this in the exercise description. While it isn't said at first that the function should only work for positive numbers, having a test that fails when a negative number is passed might have been intentional. It requires a user to have to refactor their function to continually pass the tests that may be added as edge cases are found.
So a function that just works with any numbers passed in at first is good as long as the first test passes, then test 2 and test 3. Once the user gets to test 4, it's kind of like, "Oh, we've found out we only need this function to work for positive integers and to error when any negative integer is passed in. Let me refactor my function so this test will pass now..." Which can be how writing functions and tests work in a real world scenario. You may not have every edge case thought of from the start, and the original spec may not include all the information (some info may not be known at that time). Once you have something created and the person who asked for it sees it, they may realize, "Oh you know what, we actually want to prevent this thing..."
from javascript-exercises.
That sounds reasonable. In a sense, we can take the tests as part of the specs. Personally, it feels off because rather than it being a case of not enough information, it seems like a contradiction to state integers when it actually means positive integers. In addition, in the context of a homework question, I would expect the description to state exactly what is required. There was also no indication that descriptions may be incomplete, and that tests may provide "additional information", and no explanation that this was meant to teach some of the considerations that might occur in a real world scenario, so it was just confusing to me.
Of course, that is my personal opinion. It does not change the fact that the aim is to write a code that passes all the tests, and the current formulation presents no practical impediment to this goal. If the Odin team does not feel that there is a problem, I will accept this decision.
from javascript-exercises.
I definitely get your point of view. Maybe it'd be beneficial if we mentioned somewhere in the README that certain tests may require the user to refactor their code or that the description in each exercise's README may not provide the full story. I personally might be more of the opinion that it kinda gets into semantics as to whether there's significant benefit in mentioning that vs a user discovering it on their own, but I'm but one man 😆
Let me ping @TheOdinProject/javascript to see if anyone else has opinions on this, and if any other users see this issue and have opinions definitely let us know
from javascript-exercises.
Had a look through and I see sensible points on both sides.
My personal opinion is in favour of editing the README to say
Implement a function that takes 2 positive integers...
At first, I thought people would just jump to doing everything in a conditional block if both params are positive integers, or perhaps even a guard clause for negative integers, and that this would therefore not run into the "refactor now you've bumped into a failing test" scenario.
But looking at the tests, if they did this then they would still fail because the test specifically expects the string 'ERROR'
to be returned in that scenario. If they did what is described above (which would be somewhat likely), that test would receive undefined
and thus fail.
So they'd still need to refactor to return the expected string.
How does that sound?
from javascript-exercises.
I'd be good with just updating the README with that quick change. @Houndoom would you be interested in making this update? I know you didn't check the box for being interested in your original post, but in case you've changed your mind.
from javascript-exercises.
Related Issues (20)
- Bug - 04_removeFromArray: missing test case HOT 8
- Bug - Exercises : Test expecting wrong result & typo found
- the sumAll exercise solution code HOT 1
- This is 2nd preview test. HOT 1
- Exercises : Change CommonJS to ESmodule format HOT 1
- Bug - 13_caesar: Test results appear correct but Fail HOT 4
- leapYears README.md HOT 1
- Bug - : tempConversion: Doesn't skip test cases HOT 2
- Feature: Split out exercises by lesson HOT 11
- Fundamentals Part 2, Exercise 3: Instructions Rewrite HOT 7
- Fix unclear functionality definition in 09_palindromes HOT 1
- 05_sumAll: Make clear that negative numbers and non-errors should return ERROR HOT 2
- Add disclaimer about not submitting personal solutions to PULL_REQUEST_TEMPLATE.md for all exercise repos HOT 10
- Foundations: Fix the given example on Fibonacci series in README.md of Javascript exercise(Fundamentals: Part 5) HOT 10
- 07_tempConversion: task needs rephrasing regarding module.exports HOT 5
- Bug - in the authentication basics at the bcrypt.hash function the first argument says "somePassword" which makes it seem like its for a string to name the hash or something when instead it should say req.body.password as that is how the tutorials index.ejs forms password input is setup with name passord : HOT 3
- 03_reverseString: Add alternative solution HOT 8
- Add Spanish translation HOT 3
- 05_sumAll - Add alternative solution using Gauss formula HOT 1
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 javascript-exercises.